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

feat: add OTel utility function #451

Merged
merged 3 commits into from
Feb 22, 2025
Merged

feat: add OTel utility function #451

merged 3 commits into from
Feb 22, 2025

Conversation

gruebel
Copy link
Member

@gruebel gruebel commented Feb 16, 2025

This PR

  • it is mostly equivalent to the JS implementation, except I named the field body and not data, which seems to fit better to OTel

Related Issues

Fixes #448

Copy link

codecov bot commented Feb 16, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.85%. Comparing base (613388d) to head (29aafbc).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #451      +/-   ##
==========================================
+ Coverage   97.72%   97.85%   +0.12%     
==========================================
  Files          32       37       +5     
  Lines        1629     1723      +94     
==========================================
+ Hits         1592     1686      +94     
  Misses         37       37              
Flag Coverage Δ
unittests 97.85% <100.00%> (+0.12%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gruebel gruebel requested a review from beeme1mr February 17, 2025 11:43
@beeme1mr
Copy link
Member

It looks like the OTel Python sdk only supports attributes in both span and log events. @dyladan can we include value as an attribute?

@dyladan
Copy link

dyladan commented Feb 18, 2025

I agree body is a better term because it is used by the otel spec: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/data-model.md#log-and-event-record-definition

All language implementations in OTel are experimental so I would refer to spec before referring to any language terms used.

@beeme1mr
Copy link
Member

I agree body is a better term because it is used by the otel spec: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/data-model.md#log-and-event-record-definition

All language implementations in OTel are experimental so I would refer to spec before referring to any language terms used.

I've updated the JS implementation to use body.

Copy link
Member

@beeme1mr beeme1mr left a comment

Choose a reason for hiding this comment

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

Look great, thanks! Good catch on the data vs body. I was just looking at the JS implementation but body is a better choice.

@gruebel gruebel merged commit 2d1ba85 into main Feb 22, 2025
14 checks passed
@gruebel gruebel deleted the add-telemetry-func branch February 22, 2025 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add OpenTelemetry-Compatible Telemetry Utility Function
3 participants