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

Add tests of an example sampler based on attributes #568

Merged
merged 9 commits into from
Apr 2, 2023

Conversation

tsloughter
Copy link
Member

No description provided.

@codecov
Copy link

codecov bot commented Mar 30, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (3497c6d) 38.13% compared to head (b66d814) 38.13%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #568   +/-   ##
=======================================
  Coverage   38.13%   38.13%           
=======================================
  Files          61       61           
  Lines        3598     3598           
=======================================
  Hits         1372     1372           
  Misses       2226     2226           
Flag Coverage Δ
api 67.91% <ø> (ø)
elixir 18.29% <ø> (ø)
erlang 38.11% <ø> (ø)
exporter 8.11% <ø> (ø)
sdk 78.16% <ø> (ø)
zipkin 54.16% <ø> (ø)

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

Impacted Files Coverage Δ
apps/opentelemetry/src/otel_resource.erl 75.55% <ø> (ø)
apps/opentelemetry_api/src/opentelemetry.erl 80.61% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

This was mainly done to test the example code to be used on
opentelemetry.io for Sampler docs but it also does do some
useful testing of the sampler and provides some example code.
@tsloughter tsloughter force-pushed the sampler-attributes branch from f4aebe3 to 2d587cf Compare April 1, 2023 00:10
@tsloughter
Copy link
Member Author

Argh, maps:intersect was added in OTP-24. I might just make this test only run on 24+...

@tsloughter tsloughter force-pushed the sampler-attributes branch from 9c6cc27 to c82a389 Compare April 1, 2023 10:54
@tsloughter
Copy link
Member Author

Eh, I made it a more efficient matching instead of changing it to only run on 24+. Just means now I need to update the website docs.


has_match(A, B) ->
I = maps:iterator(A),
has_match_(maps:next(I), B).
Copy link
Member

Choose a reason for hiding this comment

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

Would it be faster if you picked the smallest map to iterate from to compare with the other, since you're going for exact matches? Something like:

{Min,Max} = if maps:size(A) < maps:size(B) -> {A, B};
               true -> {B, A}
end,
I = maps:iterator(Min),
has_match_(maps:next(I), Max).

That'd ensure always having a constantly shorter iteration, though I figured the configured map won't be too big and will often be smallest.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea. I wasn't worried about performance. That's why it was originally using intersection.

Copy link
Member Author

Choose a reason for hiding this comment

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

But may as well since people may actually use this in their own sampler, so I updated it.

@tsloughter tsloughter merged commit a203823 into open-telemetry:main Apr 2, 2023
@tsloughter tsloughter deleted the sampler-attributes branch April 2, 2023 21:10
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.

2 participants