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

move attribute smodule to api #618

Merged
merged 1 commit into from
Aug 30, 2023

Conversation

tsloughter
Copy link
Member

No description provided.

@tsloughter tsloughter requested a review from a team August 29, 2023 10:42
@codecov
Copy link

codecov bot commented Aug 29, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (6d08186) 72.49% compared to head (d0d52dd) 72.49%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #618   +/-   ##
=======================================
  Coverage   72.49%   72.49%           
=======================================
  Files          61       61           
  Lines        1923     1923           
=======================================
  Hits         1394     1394           
  Misses        529      529           
Flag Coverage Δ
api 69.64% <ø> (+0.59%) ⬆️
elixir 17.47% <ø> (-0.52%) ⬇️
erlang 73.79% <ø> (ø)
exporter 66.66% <ø> (ø)
sdk 77.70% <ø> (-0.27%) ⬇️
zipkin 54.16% <ø> (ø)

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

Files Changed Coverage Δ
apps/opentelemetry_api/src/otel_attributes.erl 90.00% <ø> (ø)

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

@bryannaegele
Copy link
Contributor

What's prompting the move?

@tsloughter
Copy link
Member Author

@bryannaegele it was in the wrong place. Not sure why it was ever in the SDK. But motivating it to happen now is the desire to support #attributes as the argument to stuff like start_span so the user can process the Attributes only once -- whether that is to use the same attribute set in spans or in both a span, metric and log.

@albertored
Copy link
Contributor

At the moment attributes are stored (in spans and metrics ets tables) as provided by the user, should we cast them to attributes record before saving them?

@tsloughter
Copy link
Member Author

@albertored not sure what you mean. In the ets table the attributes are the record.

@tsloughter tsloughter merged commit d88504e into open-telemetry:main Aug 30, 2023
@tsloughter tsloughter deleted the attributes-to-api branch August 30, 2023 10:40
@albertored
Copy link
Contributor

@tsloughter my bad, span ETS table contains indeed attributes as records but the same is not true for metrics, there attributes are stored as raw maps. I think we should do the same for metrics (storing the record), I can open a PR for that if you think is right

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants