- Notifications
You must be signed in to change notification settings - Fork6.3k
Otel#21145
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Otel#21145
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
This PR enhances the OpenTelemetry plugin documentation by expanding configuration details, adding troubleshooting guidance, and restructuring the map.csv hierarchy. The changes provide comprehensive setup instructions for metrics and logs ingestion via OTLP/gRPC.
Key Changes:
- Added detailed documentation for metrics and logs configuration, including rotation/retention policies
- Included protocol support clarification (gRPC only) and SDK configuration examples
- Simplified map.csv hierarchy by removing nested "OpenTelemetry" category
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/crates/jf/otel-plugin/README.md | Comprehensive rewrite with new sections for protocol support, logs configuration, debugging, and SDK examples |
| docs/.map/map.csv | Added Windows installation entry and flattened OpenTelemetry category structure |
Tip: Customize your code reviews with copilot-instructions.md.Create the file orlearn how to get started.
Uh oh!
There was an error while loading.Please reload this page.
| instrumentation_scope_version: v0\.112\..* | ||
| # Match metric name (regex pattern, required) | ||
| metric_name: system\.network\.connections |
CopilotAIOct 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The regex pattern escapes the dot in 'system.network' but the example on line 209 uses 'system\.network\.connections' without explaining why dots need escaping. Add a note explaining that dots must be escaped in regex patterns to match literal dots rather than any character.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
without explaining why dots need escaping
Because there is no explanation andno need.
Uh oh!
There was an error while loading.Please reload this page.
vkalintiris left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Documentation-wise LGTM. However, logs are not supported yet on nightlies. This will land in a 2-3 weeks.
kanelatechnical commentedOct 15, 2025
how about we leave this unmerged until then? |
@vkalintiris hey, please check enhanced documentation + map.csv edit (currently initial otel page is empty, no reason for a clickdown)