Skip to content

Conversation

@jaydeluca
Copy link
Member

No description provided.

@jaydeluca jaydeluca requested a review from a team as a code owner October 16, 2025 21:02
if (resource == null) {
return;
}
String path = Paths.get(resource.getPath()).toString();
Copy link
Contributor

Choose a reason for hiding this comment

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

This won't work when the path contains spaces as in url they'd be encoded as %20. resource.toURI().getPath() should fix that. To me using this.getClass().getClassLoader().getResource("") feels a bit random as it is hard to explain why it should do what you want. It isn't really well defined what class loaders should return for "" and it happening to be the first directory in app loader not something from the parent loader feels more like a coincidence. Idk, maybe new java.io.File("").getAbsoluteFile() wold also work.

private static final String TMP_DIR = ".telemetry";
private static final Pattern MODULE_PATTERN =
Pattern.compile("(.*?/instrumentation/.*?)(/javaagent/|/library/)");
Pattern.compile("(.*?/instrumentation/.*?)(/javaagent|/library|/testing)");
Copy link
Member

Choose a reason for hiding this comment

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

or was remove the trailing / intentional?

Suggested change
Pattern.compile("(.*?/instrumentation/.*?)(/javaagent|/library|/testing)");
Pattern.compile("(.*?/instrumentation/.*?)(/javaagent/|/library/|/testing/)");

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, with the change in the way we are now getting the path, the result is different now

Copy link
Member

Choose a reason for hiding this comment

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

how are you thinking to use the telemetry emitted from the smoke tests?

Copy link
Member Author

@jaydeluca jaydeluca Oct 18, 2025

Choose a reason for hiding this comment

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

kafka connect only runs smoke tests, so this was the only way to get that telemetry. I don't think there will be too many other instrumentations this is needed for

…emetry-java-instrumentation into kafka-connect-metadata
@laurit laurit enabled auto-merge (squash) October 20, 2025 12:18
@laurit laurit merged commit 9975bd2 into open-telemetry:main Oct 20, 2025
81 checks passed
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.

3 participants