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.

Sign up for GitHub

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

Jump to bottom

log: Implement Value and KeyValue types #4949

Merged
merged 3 commits into from Feb 21, 2024
Merged

log: Implement Value and KeyValue types #4949

merged 3 commits into from Feb 21, 2024

Conversation

MrAlias
Copy link
Contributor

@MrAlias MrAlias commented Feb 20, 2024

Resolve #4914

@MrAlias MrAlias added area:logs Part of OpenTelemetry logs Skip Changelog PRs that do not require a CHANGELOG.md entry labels Feb 20, 2024
@codecov Codecov
Copy link

codecov bot commented Feb 20, 2024 •
edited

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (f793a05) 82.6% compared to head (f3202a4) 82.9%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #4949     +/-   ##
=======================================
+ Coverage   82.6%   82.9%   +0.2%     
=======================================
  Files        238     238             
  Lines      19038   19182    +144     
=======================================
+ Hits       15742   15903    +161     
+ Misses      3005    2987     -18     
- Partials     291     292      +1
Files Coverage Δ
log/keyvalue.go 96.5% <96.5%> (+66.5%) ⬆️

... and 1 file with indirect coverage changes

@MrAlias MrAlias force-pushed the add-log-attr-impl branch 4 times, most recently from e2e6ea3 to 755c523 Compare February 20, 2024 21:31
@MrAlias MrAlias marked this pull request as ready for review February 20, 2024 22:01
This was referenced Feb 20, 2024
Closed
Closed
pellared
pellared approved these changes Feb 20, 2024
View reviewed changes
Copy link
Member

@pellared pellared left a comment •
edited

Choose a reason for hiding this comment

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

LGTM.

EDIT: All my comments should be addressed as separate issues/PRs.

log/keyvalue_test.go Show resolved Hide resolved
log/keyvalue_test.go Show resolved Hide resolved
log/keyvalue.go Show resolved Hide resolved
This was referenced Feb 21, 2024
Closed
Closed
Closed
@pellared pellared changed the title Implement Value and KeyValue in types otel/log log: Implement Value and KeyValue in types otel/log` Feb 21, 2024
@pellared pellared changed the title log: Implement Value and KeyValue in types otel/log` log: Implement Value and KeyValue types Feb 21, 2024
dashpole
dashpole reviewed Feb 21, 2024
View reviewed changes
log/go.mod Show resolved Hide resolved
This was referenced Feb 21, 2024
Merged
Merged
@MrAlias MrAlias merged commit 7b3382e into open-telemetry:main Feb 21, 2024
30 checks passed
@MrAlias MrAlias deleted the add-log-attr-impl branch February 21, 2024 21:20
@MrAlias MrAlias added this to the v1.24.0 milestone Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pellared pellared pellared approved these changes

@dmathieu dmathieu dmathieu approved these changes

@dashpole dashpole dashpole approved these changes

@hanyuancheung hanyuancheung hanyuancheung approved these changes

@Aneurysm9 Aneurysm9 Awaiting requested review from Aneurysm9

@evantorrie evantorrie Awaiting requested review from evantorrie evantorrie is a code owner

@XSAM XSAM Awaiting requested review from XSAM XSAM is a code owner

@MadVikingGod MadVikingGod Awaiting requested review from MadVikingGod MadVikingGod is a code owner

Assignees
No one assigned
Labels
area:logs Part of OpenTelemetry logs Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
Go: Logs (GA)
Status: Done
Milestone
v1.24.0/v0.46.0
Development

Successfully merging this pull request may close these issues.

Implement the Value and KeyValue types
5 participants
@MrAlias @dmathieu @dashpole @pellared @hanyuancheung