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

NH-97250: upgrade to upstream v2.10.0 #285

Merged
merged 2 commits into from
Dec 9, 2024
Merged

NH-97250: upgrade to upstream v2.10.0 #285

merged 2 commits into from
Dec 9, 2024

Conversation

cleverchuk
Copy link
Contributor

Removed test because it's producing a false-positive. Existence of that log line doesn't necessarily mean that the class is being instrumented. For an instrumentation to actually apply to our classes it must be explicitly targeted to the relocation namespace of com.solarwinds.joboe.* and we have tests that ensure dependencies are properly relocated to that namespace.

@cleverchuk cleverchuk requested a review from a team as a code owner December 6, 2024 18:38
Copy link
Contributor

@cheempz cheempz left a comment

Choose a reason for hiding this comment

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

LGTM and thanks for the follow-ons you noted regarding gradle convention plugin @cleverchuk. Tangent: the otel sdk change makes me wonder, are we truncating the custom transaction name to 255 chars?

@cleverchuk
Copy link
Contributor Author

LGTM and thanks for the follow-ons you noted regarding gradle convention plugin @cleverchuk. Tangent: the otel sdk change makes me wonder, are we truncating the custom transaction name to 255 chars?

yessum, here. the sdk one was added because initially the lambda wouldn't go through the normal name generation path. that's has changed since the use of onEnding api. the one line that was removed was a vestigial of the past.

@cleverchuk cleverchuk merged commit c97a2b0 into main Dec 9, 2024
13 checks passed
@cleverchuk cleverchuk deleted the cc/NH-97250 branch December 9, 2024 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants