Skip to content

Conversation

@xiang17
Copy link
Contributor

@xiang17 xiang17 commented Jun 6, 2025

Fixes #
Design discussion issue #

Changes

Please provide a brief description of the changes here.

HTTP semconv: Combination of url.scheme, server.address, server.port, url.path and url.query attributes for HTTP server spans.

Merge requirement checklist

  • CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
  • Unit tests added/updated
  • Appropriate CHANGELOG.md files updated for non-trivial changes
  • Changes in public API reviewed (if applicable)

@xiang17 xiang17 requested a review from a team as a code owner June 6, 2025 11:11
@github-actions github-actions bot added the comp:exporter.geneva Things related to OpenTelemetry.Exporter.Geneva label Jun 6, 2025
@codecov
Copy link

codecov bot commented Jun 6, 2025

Codecov Report

Attention: Patch coverage is 98.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 53.93%. Comparing base (71655ce) to head (7333b72).
Report is 874 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...er.Geneva/Internal/MsgPack/MsgPackTraceExporter.cs 98.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #2818       +/-   ##
===========================================
- Coverage   73.91%   53.93%   -19.98%     
===========================================
  Files         267       55      -212     
  Lines        9615     5141     -4474     
===========================================
- Hits         7107     2773     -4334     
+ Misses       2508     2368      -140     
Flag Coverage Δ
unittests-Exporter.Geneva 53.93% <98.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...er.Geneva/Internal/MsgPack/MsgPackTraceExporter.cs 93.65% <98.00%> (ø)

... and 251 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@rajkumar-rangaraj rajkumar-rangaraj requested a review from Copilot June 6, 2025 16:49
Copy link
Contributor

Copilot AI left a 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 adds support for composing an httpUrl for HTTP server spans by combining multiple HTTP semantic convention attributes. Key changes include adding unit tests for HTTP URL composition in both unstable and stable semantic convention scenarios, updating Geneva exporter tests, and extending the MsgPackTraceExporter to cache and build the HTTP URL based on new mappings.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
test/OpenTelemetry.Exporter.Geneva.Tests/MsgPackTraceExporterTests.cs Tests validating httpUrl parts caching and URL generation
test/OpenTelemetry.Exporter.Geneva.Tests/GenevaTraceExporterTests.cs Integration tests for verifying HTTP URL mapping in server and client spans
src/OpenTelemetry.Exporter.Geneva/Internal/MsgPack/MsgPackTraceExporter.cs Updates to caching mechanisms and HTTP URL composition logic using new mapping dictionaries
src/OpenTelemetry.Exporter.Geneva/CHANGELOG.md Changelog updated with new HTTP URL mapping feature
Comments suppressed due to low confidence (2)

src/OpenTelemetry.Exporter.Geneva/Internal/MsgPack/MsgPackTraceExporter.cs:489

  • [nitpick] Consider renaming the variable 'httpMethodString' (assigned at this point) to 'httpMethodTagKey' to more explicitly indicate that it holds the tag key rather than the HTTP method value.
if (isServerActivity && replacementKey == "httpMethod")

src/OpenTelemetry.Exporter.Geneva/Internal/MsgPack/MsgPackTraceExporter.cs:462

  • [nitpick] Consider adding an inline comment here to clarify the initialization and reset behavior of the 'HttpUrlParts' thread-local variable to improve code maintainability.
var httpUrlParts = this.HttpUrlParts.Value ?? new object?[CS40_PART_B_HTTPURL_MAPPING.Count];

Copy link
Member

@rajkumar-rangaraj rajkumar-rangaraj left a comment

Choose a reason for hiding this comment

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

LGTM, there is a minor style error please fix it.

This was referenced Oct 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:exporter.geneva Things related to OpenTelemetry.Exporter.Geneva

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants