Skip to content

Conversation

jaydeluca
Copy link
Member

@jaydeluca jaydeluca commented May 21, 2025

Now we're getting to the fun part...

This PR covers a few changes:

  • Added the otel.nullaway-conventions plugin (as recommended here) and addressed all issues raised by it
  • Added an interceptor in the test runner to collect metrics emitted during runs (when flag enabled) and writes them to .telemetry files within each instrumentation module directory
  • Updated the instrumentation analyzer to look for these files and populate the module with the information
  • Updated the YamlParser to write these metrics to the instrumentation-list.yaml file
  • Adds a helper script that will target and run the tests only for the modules we have configured, and will clean up the temp files after completion

I wanted to keep this PR small-ish, so I didn't go through and start enabling this for other modules, but I will followup with that.

I am also going to play around with how we might handle situations where different metrics are emitted by different config options (like -Dotel.semconv-stability.opt-in=database, as an example), and how we might best display that information

Related to #13468

@jaydeluca jaydeluca requested a review from a team as a code owner May 21, 2025 11:15

# Remove all .telemetry directories recursively from the project root
echo "Searching for and removing all .telemetry directories..."
find . -type d -name ".telemetry" -exec rm -rf {} +
Copy link
Member Author

Choose a reason for hiding this comment

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

@trask - from your question during the SIG meeting:

The {} bit is the placeholder for the exec command. Whatever files are found by find are inserted in place of the brackets. The + means to build up a long list of the found files and call the exec on all of them at once instead of one at a time, like the more traditional -exec {} ; variant.

https://serverfault.com/questions/72978/what-does-rm-do

hs_err_pid*
replay_pid*
.attach_pid*
**/.telemetry*
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
**/.telemetry*
.telemetry/

Copy link
Member Author

Choose a reason for hiding this comment

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

there is a directory generated per instrumentation module

image

Copy link
Member

@trask trask May 30, 2025

Choose a reason for hiding this comment

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

it should work

if you only want to ignore it in the root, it would be different:

/.telemetry/

@trask trask merged commit 94d2c0f into open-telemetry:main May 29, 2025
88 checks passed
@jaydeluca jaydeluca mentioned this pull request May 29, 2025
24 tasks
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.

2 participants