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

doc(k8s-operator): document customizing enabled auto-instrumentations for nodejs with kubernetes-operator #4137

Conversation

atsu85
Copy link
Contributor

@atsu85 atsu85 commented Mar 12, 2024

The change

Current documentation states that it isn't possible to disable specific nodejs auto-instrumentation packages when using kubernetes-operator, but that isn't true after next kubernetes operator release, that allows enabling instrumentations based on OTEL_NODE_ENABLED_INSTRUMENTATIONS environment variable.

[UPDATE:] Dependancy PRs released for now

This documentation change is caused by feature enabled by merged PR
that currently isn't released (hence the DRAFT status of this PR),
and another PR that built the foundation for it.

Copy link
Member

@svrnm svrnm left a comment

Choose a reason for hiding this comment

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

Thank you @atsu85, please see my comment on updating the Node.JS documentation first, let me know if you have bandwidth to do the same as well.

content/en/docs/kubernetes/operator/automatic.md Outdated Show resolved Hide resolved
content/en/docs/kubernetes/operator/automatic.md Outdated Show resolved Hide resolved
@svrnm
Copy link
Member

svrnm commented Apr 11, 2024

@atsu85 any updates, can you take a look at my feedback? feel free to ask questions if you are unsure how to proceed

@atsu85 atsu85 force-pushed the 4130-doc-k8s-operator-nodejs-customizing-enabled-auto-instrumentations branch 3 times, most recently from beab727 to 31aa091 Compare April 11, 2024 11:32
@atsu85
Copy link
Contributor Author

atsu85 commented Apr 11, 2024

can you take a look at my feedback?

@svrnm, sorry for noticing this so late and thanks for the feedback.

Please take a look, I hope my solution is OK.

@atsu85 atsu85 marked this pull request as ready for review April 11, 2024 11:36
@atsu85 atsu85 requested review from a team April 11, 2024 11:36
Copy link
Contributor

@swiatekm swiatekm left a comment

Choose a reason for hiding this comment

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

LGTM

@atsu85 atsu85 force-pushed the 4130-doc-k8s-operator-nodejs-customizing-enabled-auto-instrumentations branch from 31aa091 to 30c355a Compare April 11, 2024 15:04
@atsu85 atsu85 requested a review from svrnm April 11, 2024 15:08
@atsu85 atsu85 force-pushed the 4130-doc-k8s-operator-nodejs-customizing-enabled-auto-instrumentations branch from 30c355a to 92478ad Compare April 11, 2024 16:02
@atsu85
Copy link
Contributor Author

atsu85 commented Apr 11, 2024

FYI: amended commits to fix linting errors (only includes changes done by npm run fix:format)

@atsu85 atsu85 force-pushed the 4130-doc-k8s-operator-nodejs-customizing-enabled-auto-instrumentations branch from ad12e34 to 87a3835 Compare April 12, 2024 05:11
@atsu85
Copy link
Contributor Author

atsu85 commented Apr 12, 2024

@svrnm , please approve this PR again!

I saw that Spelling check action was failing, because of nestjs in the link title:
image
so I replaced it with @opentelemetry/instrumentation-express, in hope that it shouldn't fail spell check action and rebased&squashed it on top of latest main branch.

@svrnm svrnm merged commit 1a20a31 into open-telemetry:main Apr 12, 2024
14 checks passed
@svrnm
Copy link
Member

svrnm commented Apr 12, 2024

Thank you @atsu85

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.

5 participants