Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing bridge API bug #6113

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

juliuskoval
Copy link
Contributor

Fixes #
Design discussion issues #4433, #6109

Changes

This would change the behavior of the logging bridge API where body of the exported log would be set by the Body of the emitted LogRecordData. I'm not sure if this change would be OK with you, but I don't see another way of directly setting both the body and the {OriginalFormat} attribute. This workaround is possible, but obviously not ideal. This scenario wasn't covered by unit tests, so I added one.

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)

@juliuskoval juliuskoval requested a review from a team as a code owner January 29, 2025 20:00
@github-actions github-actions bot added pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package labels Jan 29, 2025
@CodeBlanch CodeBlanch marked this pull request as draft January 29, 2025 20:24
@CodeBlanch
Copy link
Member

I think we need to agree on a direction via the issue before trying to fix something.

Copy link
Contributor

github-actions bot commented Feb 6, 2025

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day.

@github-actions github-actions bot added the Stale Issues and pull requests which have been flagged for closing due to inactivity label Feb 6, 2025
Copy link

codecov bot commented Feb 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.38%. Comparing base (89ea5c6) to head (96eeaba).
Report is 5 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6113      +/-   ##
==========================================
- Coverage   86.39%   86.38%   -0.01%     
==========================================
  Files         257      257              
  Lines       11640    11635       -5     
==========================================
- Hits        10056    10051       -5     
  Misses       1584     1584              
Files with missing lines Coverage Δ
...ementation/Serializer/ProtobufOtlpLogSerializer.cs 98.43% <ø> (-0.06%) ⬇️
src/OpenTelemetry/Logs/LoggerSdk.cs 100.00% <ø> (ø)

@github-actions github-actions bot removed the Stale Issues and pull requests which have been flagged for closing due to inactivity label Feb 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants