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

Add trace attributes to the log entry #3018

Merged
merged 2 commits into from
Apr 15, 2021
Merged

Add trace attributes to the log entry #3018

merged 2 commits into from
Apr 15, 2021

Conversation

alexvanboxel
Copy link
Contributor

@alexvanboxel alexvanboxel commented Apr 7, 2021 •
edited

Description:
Add trace, span, and flag attributes to the log entry.

Testing:
Added tests for trace, span, and flags

@alexvanboxel alexvanboxel requested review from djaglowski and a team as code owners April 7, 2021 20:30
@codecov
Copy link

codecov bot commented Apr 7, 2021 •
edited

Codecov Report

Merging #3018 (59b4737) into main (fbef6d4) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3018      +/-   ##
==========================================
+ Coverage   91.59%   91.61%   +0.01%     
==========================================
  Files         486      486              
  Lines       23515    23528      +13     
==========================================
+ Hits        21539    21554      +15     
+ Misses       1469     1464       -5     
- Partials      507      510       +3
Flag Coverage Δ
integration 63.30% <ø> (ø)
unit 90.61% <100.00%> (+0.01%) ⬆️

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

Impacted Files Coverage Δ
internal/stanza/converter.go 95.54% <100.00%> (+0.24%) ⬆️
receiver/k8sclusterreceiver/watcher.go 95.29% <0.00%> (-2.36%) ⬇️
exporter/datadogexporter/metadata/metadata.go 90.14% <0.00%> (+5.63%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fbef6d4...59b4737. Read the comment docs.

@djaglowski
Copy link
Member

@alexvanboxel Looks like we duplicated some work (#3017). The new test is great though. If mine is not merged by the time you circle back to this, then feel free to borrow any changes and I'll close mine.

@tigrannajaryan
Copy link
Member

I merged #3017

@djaglowski
Copy link
Member

Thanks Tigran.

@alexvanboxel Will you please rebase so we can pick up the new test?

@alexvanboxel alexvanboxel changed the title Update the log-collector library to v0.17.0 Add trace attributes to the log entry Apr 8, 2021
@alexvanboxel
Copy link
Contributor Author

@djaglowski all green. I had to re-push though (without some code change) to get the build green. It seems we have some flaky tests.

djaglowski
djaglowski approved these changes Apr 9, 2021
View reviewed changes
Copy link
Member

@djaglowski djaglowski left a comment

Choose a reason for hiding this comment

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

LGTM

tigrannajaryan
tigrannajaryan reviewed Apr 9, 2021
View reviewed changes
internal/stanza/converter.go Show resolved Hide resolved
internal/stanza/converter.go Outdated Show resolved Hide resolved
@tigrannajaryan
Copy link
Member

Please resolve the merge conflict.

@alexvanboxel alexvanboxel force-pushed the feature/log17 branch 2 times, most recently from 43f0495 to bad405f Compare April 9, 2021 21:24
@alexvanboxel
Copy link
Contributor Author

Rebased and green, I'll have a look at pushing down the fixed size TraceId and SpanId to the log-collector

@alexvanboxel
Add trace attributes to the log entry
608831e 
JSON log parsers are now able to extract the OpenTelemetry compliant
trace_id, span_id and trace_flags attributes and add them to the
correct attributes.
@alexvanboxel
Add comment on TraceFlag to flag masking
59b4737
@alexvanboxel
Copy link
Contributor Author

@tigrannajaryan is the explanation enough? Can I merge?

tigrannajaryan
tigrannajaryan approved these changes Apr 15, 2021
View reviewed changes
Copy link
Member

@tigrannajaryan tigrannajaryan left a comment

Choose a reason for hiding this comment

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

Thanks @alexvanboxel

@tigrannajaryan tigrannajaryan merged commit 02ab2b4 into open-telemetry:main Apr 15, 2021
pmatyjasek-sumo pushed a commit to pmatyjasek-sumo/opentelemetry-collector-contrib that referenced this pull request Apr 28, 2021
@alexvanboxel @pmatyjasek-sumo
Add trace attributes to the log entry (open-telemetry#3018)
d703dd1 
JSON log parsers are now able to extract the OpenTelemetry compliant
trace_id, span_id and trace_flags attributes and add them to the
correct attributes.
mstumpfx pushed a commit to mstumpfx/opentelemetry-collector-contrib that referenced this pull request Aug 31, 2021
@alexvanboxel
Add trace attributes to the log entry (open-telemetry#3018)
8b2e30a 
JSON log parsers are now able to extract the OpenTelemetry compliant
trace_id, span_id and trace_flags attributes and add them to the
correct attributes.
@alexvanboxel alexvanboxel mentioned this pull request Sep 15, 2023
Closed
6 tasks
@alexvanboxel alexvanboxel deleted the feature/log17 branch November 16, 2023 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tigrannajaryan tigrannajaryan tigrannajaryan approved these changes

@djaglowski djaglowski Awaiting requested review from djaglowski

Assignees

@kbrockhoff kbrockhoff

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@alexvanboxel @djaglowski @tigrannajaryan @kbrockhoff