Skip to content

Conversation

@rajkumar-rangaraj
Copy link
Member

Fixes #
Design discussion issue #

The AFDCorrelationIdLogProcessor was throwing unhandled exceptions when attempting to retrieve the AFDCorrelationId from RuntimeContext if the value had not been previously set by customers. This caused processing failures in the telemetry pipeline.
Solution.

Changes

Please provide a brief description of the changes here.

  • Added a state tracking mechanism using GenevaAfdCorrelationIdStateTracker to remember when access failures occur
  • Created a RuntimeContextSlot to track correlation ID access state
  • Added try-catch block in GetRuntimeContextValue() to handle exceptions gracefully
  • Implemented a check to skip attempting to access the correlation ID if previous attempts failed
  • Added proper resource cleanup in the Dispose method

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)

@rajkumar-rangaraj rajkumar-rangaraj requested a review from a team as a code owner April 22, 2025 00:42
@github-actions github-actions bot added the comp:exporter.geneva Things related to OpenTelemetry.Exporter.Geneva label Apr 22, 2025
@codecov
Copy link

codecov bot commented Apr 22, 2025

Codecov Report

Attention: Patch coverage is 56.52174% with 10 lines in your changes missing coverage. Please review.

Project coverage is 52.84%. Comparing base (71655ce) to head (3266416).
Report is 809 commits behind head on main.

Files with missing lines Patch % Lines
...er.Geneva/Internal/AFDCorrelationIdLogProcessor.cs 72.22% 5 Missing ⚠️
...ry.Exporter.Geneva/Internal/ExporterEventSource.cs 0.00% 5 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #2708       +/-   ##
===========================================
- Coverage   73.91%   52.84%   -21.08%     
===========================================
  Files         267       55      -212     
  Lines        9615     5079     -4536     
===========================================
- Hits         7107     2684     -4423     
+ Misses       2508     2395      -113     
Flag Coverage Δ
unittests-Exporter.Geneva 52.84% <56.52%> (?)

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

Files with missing lines Coverage Δ
...er.Geneva/Internal/AFDCorrelationIdLogProcessor.cs 81.25% <72.22%> (ø)
...ry.Exporter.Geneva/Internal/ExporterEventSource.cs 16.66% <0.00%> (+11.90%) ⬆️

... and 251 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dmitry-kharchenko
Copy link

Overall LGTM

Comment on lines +163 to +165
// Check that no exceptions were thrown
// If our implementation is correct, logs from threads without correlation ID
// should have been processed without exceptions
Copy link
Contributor

Choose a reason for hiding this comment

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

is this meant to be a TODO?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a statement indicating that no exceptions were thrown. Since the existing implementation threw an exception, I had to add it, instead of making test complex.

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.

4 participants