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

Add otlploghttp transform package #5191

Merged
merged 10 commits into from
Apr 12, 2024

Conversation

MrAlias
Copy link
Contributor

@MrAlias MrAlias commented Apr 10, 2024

Part of #5057

This does not include testing for Resource or Scope values. See #5192.

@MrAlias MrAlias added area:logs Part of OpenTelemetry logs Skip Changelog PRs that do not require a CHANGELOG.md entry pkg:exporter:otlp Related to the OTLP exporter package labels Apr 10, 2024
Copy link

codecov bot commented Apr 10, 2024

Codecov Report

Attention: Patch coverage is 94.61538% with 14 lines in your changes are missing coverage. Please review.

Project coverage is 84.3%. Comparing base (7da00d9) to head (663db5c).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #5191     +/-   ##
=======================================
+ Coverage   84.1%   84.3%   +0.1%     
=======================================
  Files        258     258             
  Lines      16713   16966    +253     
=======================================
+ Hits       14067   14305    +238     
- Misses      2352    2362     +10     
- Partials     294     299      +5     
Files Coverage Δ
exporters/otlp/otlplog/otlploghttp/client.go 100.0% <100.0%> (+12.5%) ⬆️
exporters/otlp/otlplog/otlploghttp/exporter.go 83.3% <50.0%> (-8.4%) ⬇️
...otlp/otlplog/otlploghttp/internal/transform/log.go 95.2% <95.2%> (-4.8%) ⬇️

... and 1 file with indirect coverage changes

@MrAlias MrAlias changed the title Add otlploghttp transform pkg Add otlploghttp transform package Apr 10, 2024
Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

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

LGTM. Just one comment that I find worth addressing.

exporters/otlp/otlplog/otlploghttp/exporter.go Outdated Show resolved Hide resolved
@MrAlias MrAlias mentioned this pull request Apr 11, 2024
15 tasks
Copy link
Member

@XSAM XSAM left a comment

Choose a reason for hiding this comment

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

We might need to move the implementation of transform to a shared place. But we can do that as parts of #5056

BTW, this PR seems to be assigned to a wrong project Go Auto Instrumentation: Beta (You might need to move it out of the project, I don't have the write permission of this project)

@MrAlias MrAlias merged commit c971888 into open-telemetry:main Apr 12, 2024
27 checks passed
@MrAlias MrAlias deleted the otlploghttp-transform branch April 12, 2024 14:28
@MrAlias MrAlias added this to the v1.26.0 milestone Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:logs Part of OpenTelemetry logs pkg:exporter:otlp Related to the OTLP exporter package Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants