Skip to content

Run build without warnings#1387

Merged
trask merged 4 commits into
open-telemetry:masterfrom
iNikem:check
Oct 15, 2020
Merged

Run build without warnings#1387
trask merged 4 commits into
open-telemetry:masterfrom
iNikem:check

Conversation

@iNikem

@iNikem iNikem commented Oct 14, 2020

Copy link
Copy Markdown
Contributor

Fixes #984

Required test coverage levels are very relaxed atm. We need to address them separately.

trask
trask approved these changes Oct 15, 2020
View reviewed changes

@trask trask left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the reason for the strange io.opentelemetry.javaagent.instrumentation.api.jdbc package was as an optimization DataDog/dd-trace-java#1208.

i'm good with moving it back to jdbc instrumentation module.

you'll need to add those classes back into helperClassNames to make muzzle happy.

makes sense to do the same with io.opentelemetry.javaagent.instrumentation.api.rmi

@iNikem

iNikem commented Oct 15, 2020

Copy link
Copy Markdown
Contributor Author

the reason for the strange io.opentelemetry.javaagent.instrumentation.api.jdbc package was as an optimization DataDog/dd-trace-java#1208.

Is this one more piece of tribal knowledge about muzzle? I was not aware that muzzle's performance may depend on our own class location.

@mateuszrzeszutek

mateuszrzeszutek commented Oct 15, 2020

Copy link
Copy Markdown
Member

i'm good with moving it back to jdbc instrumentation module.

👍

I think that we'll gain more performance in muzzle by implementing #1233 anyway.

@iNikem iNikem closed this Oct 15, 2020
@iNikem iNikem reopened this Oct 15, 2020
@trask

trask commented Oct 15, 2020

Copy link
Copy Markdown
Member

Is this one more piece of tribal knowledge about muzzle? I was not aware that muzzle's performance may depend on our own class location.

everything in helperClassNames() is injected into the user's class loader, which is not free

@trask trask merged commit 15e0b2f into open-telemetry:master Oct 15, 2020
@iNikem iNikem deleted the check branch October 16, 2020 04:34
@trask trask mentioned this pull request Oct 26, 2020
Closed
schmikei pushed a commit to schmikei/opentelemetry-java-instrumentation that referenced this pull request Apr 17, 2025
@jsuereth
Minor dev-experience fixes for making spec changes. (open-telemetry#1387
47fadaa 
)

* Minor dev-experience fixes for making spec changes.

* Fixes from review.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@trask trask trask approved these changes

@jkwatson jkwatson Awaiting requested review from jkwatson

@tylerbenson tylerbenson Awaiting requested review from tylerbenson

+2 more reviewers

@anuraaga anuraaga anuraaga approved these changes

@mateuszrzeszutek mateuszrzeszutek mateuszrzeszutek approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

gradle build and gradle check are failing

4 participants

@iNikem @mateuszrzeszutek @trask @anuraaga