-
Notifications
You must be signed in to change notification settings - Fork 471
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
using opentelemetry.io* namespace for extracing resource based on annotations from k8s Api #2330 #3010
using opentelemetry.io* namespace for extracing resource based on annotations from k8s Api #2330 #3010
Conversation
Signed-off-by: Horiodino <holiodin@gmail.com>
The title of your PRs indicates it's adding support for attribute labels as well resource ones, but the code only seems to implement resources. Are you planning on a follow-up? |
oh sorry for that , i have just added anotation for now because annotations are for attaching non-identifying metadata. |
I think we should only do from annotations and not labels. @Horiodino can you add a changelog and update the title to specify only annotations? |
done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, small nitpick about naming. Please add a changelog entry as well, you can run make chlog-new
to have the tooling generate a template for you.
Signed-off-by: Horiodino <holiodin@gmail.com>
@Horiodino last thing to do here is add a changelog via |
@jaronoff97 hearding about changelog first time ! i added it now |
7522f27
to
9e16256
Compare
9e16256
to
5e7e356
Compare
Signed-off-by: Horiodino <holiodin@gmail.com>
5e7e356
to
c69bf0e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we please document this in the root readme?
@Horiodino when you get a chance, if you could add some documentation into the README.md then we should be able to merge this! 🙇 |
…urce Signed-off-by: Horiodino <holiodin@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested some phrasing changes to the documentation.
Co-authored-by: Mikołaj Świątek <mail@mikolajswiatek.com>
Co-authored-by: Mikołaj Świątek <mail@mikolajswiatek.com>
@Horiodino thank you for your contribution :D |
change_type: sdk.go
Modified the logic for iterating over pod annotations, utilizing pod.GetAnnotations() to identify and extract resources using the specified namespace prefix (constants.ReservedNamespace).
Implemented the use of the OpenTelemetry.io namespace for extracting resource annotations from Kubernetes pods.
Enhanced handling to streamline the extraction process and avoid overwriting existing resource mappings.
issues: [https://github.com//issues/2181]
#2330
TestCase ✅