Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: @opentelemetry/instrumentation-express instrumentation, config story #32

Closed
3 of 4 tasks
david-luna opened this issue Jan 31, 2024 · 2 comments
Closed
3 of 4 tasks
Assignees

Comments

@david-luna
Copy link
Member

david-luna commented Jan 31, 2024

express instrumentation is already included in @elastic/opentelemetry-node but a the time of writing this it has only the default options.

new ExpressInstrumentation(),

To acknowledge the instrumentation is in place we should

  • discuss if necessary to add some defaults to this configuration
  • see if the current tests are enough for a sanity check
  • check if necessary to have our own set of environment vars to configure it without using JS code
  • start documentation and add this to the list of instrumentations
@david-luna david-luna changed the title feat: express instrumentation, config story feat: @opentelemetry/instrumentation-express instrumentation, config story Jan 31, 2024
@trentm trentm added this to the demo milestone Jan 31, 2024
@trentm trentm removed this from the demo milestone Feb 1, 2024
@trentm
Copy link
Member

trentm commented Mar 21, 2024

Now that we have getInstrumentations (https://github.com/elastic/elastic-otel-node/pull/82/files#diff-006a61fd8273284b6adc190ba00d4c2eb77d6b41b657d33d7ad480154b5f6e80R37-R40) which allows passing in config, I think we are good here. (That getInstrumentations needs docs, but that's a separate issue.)

  • discuss if necessary to add some defaults to this configuration

My vote is no, at least for now. Any defaults we feel are good for users we should push upstream.

Currently available config options are here: https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/plugins/node/opentelemetry-instrumentation-express/README.md#express-instrumentation-options

@trentm trentm self-assigned this Mar 21, 2024
@trentm
Copy link
Member

trentm commented Mar 21, 2024

  • start documentation and add this to the list of instrumentations

I think we should leave that to #4 or other general docs issues.

@trentm trentm closed this as completed Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants