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 Spring Boot service name guesser / ResourceProvider #6516

Merged
merged 11 commits into from
Sep 7, 2022

Conversation

breedx-splk
Copy link
Contributor

This is effectively the same addition that was over in open-telemetry/opentelemetry-java-contrib#427 and we agreed to move into the main instrumentation repo.

Is this the correct module/location?

This also relates to open-telemetry/opentelemetry-java#4718 because ideally we the guesser introduced in this changeset would not override other, potentially more meaningful service.name values that might have already been set by other components.

@breedx-splk breedx-splk requested a review from a team as a code owner August 26, 2022 22:56
@mateuszrzeszutek
Copy link
Member

Is this the correct module/location?

Nope, that's the spring boot autoconfigure starter code -- it's meant to be used instead of the javaagent, as a drop-in spring plugin. It's not included in the javaagent.
I think that the resource provider definitely deserves a separate module; probably somewhere in the instrumentation/spring/ dir. Something like spring-boot-resources?

Comment on lines 262 to 263
* Attempts to use ProcessHandle to get the full commandline of the current process. Will only
* succeed on java 9+.
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

@trask trask Sep 6, 2022

Choose a reason for hiding this comment

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

nm, I see this is for getting program args, maybe rename method to attemptGetProgramArgsViaReflection?

EDIT: oh, I see this is "full" commandline, so ignore my rename suggestion, maybe just a little more explicit javadoc to unconfuse those like me?

Copy link
Member

Choose a reason for hiding this comment

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

Done 👍

Mateusz Rzeszutek and others added 2 commits September 7, 2022 09:45
Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
@trask trask merged commit 56f4e52 into open-telemetry:main Sep 7, 2022
LironKS pushed a commit to helios/opentelemetry-java-instrumentation that referenced this pull request Oct 23, 2022
…ry#6516)

* Add spring boot service name guesser.

* add encoding

* improve commandline handling

* move guesser to own module

* use readAllBytes which exists in java 8

* spotless

* add note and link to spring docs

* group for readability

* repackage

* Apply suggestions from code review

Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>

* code review comments

Co-authored-by: Mateusz Rzeszutek <mrzeszutek@splunk.com>
Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
LironKS pushed a commit to helios/opentelemetry-java-instrumentation that referenced this pull request Oct 31, 2022
…ry#6516)

* Add spring boot service name guesser.

* add encoding

* improve commandline handling

* move guesser to own module

* use readAllBytes which exists in java 8

* spotless

* add note and link to spring docs

* group for readability

* repackage

* Apply suggestions from code review

Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>

* code review comments

Co-authored-by: Mateusz Rzeszutek <mrzeszutek@splunk.com>
Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
LironKS pushed a commit to helios/opentelemetry-java-instrumentation that referenced this pull request Dec 4, 2022
…ry#6516)

* Add spring boot service name guesser.

* add encoding

* improve commandline handling

* move guesser to own module

* use readAllBytes which exists in java 8

* spotless

* add note and link to spring docs

* group for readability

* repackage

* Apply suggestions from code review

Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>

* code review comments

Co-authored-by: Mateusz Rzeszutek <mrzeszutek@splunk.com>
Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
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.

None yet

3 participants