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

Add docker image containing nodejs auto instrumentation #508

Merged

Conversation

anuraaga
Copy link
Contributor

@anuraaga anuraaga commented Nov 5, 2021

Created image: https://github.com/anuraaga/opentelemetry-operator/pkgs/container/opentelemetry-operator%2Fautoinstrumentation-nodejs

I think the autoinstrumentation wrapper could be pushed to js-contrib though we'll still need to package something with Docker. Hopefully it's OK to include it here for now to try out the autoinstrumentation experience.

@pavolloffay @jpkrohling

/cc @dyladan

Would be used by #507

@anuraaga anuraaga requested review from a team and owais November 5, 2021 08:20
@@ -0,0 +1,12 @@
import { getNodeAutoInstrumentations } from '@opentelemetry/auto-instrumentations-node';
import { OTLPTraceExporter } from '@opentelemetry/exporter-otlp-grpc';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I understand NodeJS doesn't support env vars like OTEL_TRACES_EXPORTER yet. When it does, we would remove this sort of hard-coding.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be an issue on this repository then?

@anuraaga anuraaga force-pushed the nodejs-autoinstrumentation-docker branch from 46586bd to 38982a9 Compare November 5, 2021 08:33
Copy link
Member

@jpkrohling jpkrohling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea about the nodejs parts, but the github workflow is looking good. I will wait for a review from someone who knows about the nodejs parts before merging.

@@ -0,0 +1,12 @@
import { getNodeAutoInstrumentations } from '@opentelemetry/auto-instrumentations-node';
import { OTLPTraceExporter } from '@opentelemetry/exporter-otlp-grpc';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be an issue on this repository then?

@pavolloffay
Copy link
Member

pavolloffay commented Nov 5, 2021

@anuraaga how does the injection of nodeJS instrumentation to an application work? Could you please briefly describe here what the operator will have to do to enable the instrumentation automatically?

@anuraaga
Copy link
Contributor Author

anuraaga commented Nov 5, 2021

@pavolloffay The code is in #507 :) But if it would help to also have text explanation let me know

@jpkrohling jpkrohling merged commit 72ba88c into open-telemetry:main Nov 8, 2021
shree007 pushed a commit to shree007/opentelemetry-operator that referenced this pull request Dec 12, 2021
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this pull request May 1, 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

Successfully merging this pull request may close these issues.

3 participants