Skip to content

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

@codecov
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

4 participants