Skip to content

update sampler to take a context and update some names to match spec #134

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

Merged
merged 2 commits into from
Nov 2, 2020

Conversation

tsloughter
Copy link
Member

No description provided.

@codecov
Copy link

codecov bot commented Oct 31, 2020

Codecov Report

Merging #134 into master will increase coverage by 0.17%.
The diff coverage is 90.24%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #134      +/-   ##
==========================================
+ Coverage   32.13%   32.30%   +0.17%     
==========================================
  Files          63       63              
  Lines        2981     2990       +9     
==========================================
+ Hits          958      966       +8     
- Misses       2023     2024       +1     
Flag Coverage Δ
api 46.44% <100.00%> (ø)
elixir 16.24% <0.00%> (ø)
erlang 32.15% <90.24%> (+0.17%) ⬆️
exporter 17.05% <ø> (ø)
sdk 65.11% <89.74%> (+0.30%) ⬆️

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

Impacted Files Coverage Δ
...ntelemetry_exporter/src/opentelemetry_exporter.erl 61.11% <ø> (ø)
apps/opentelemetry/src/otel_sampler.erl 88.37% <80.00%> (-1.38%) ⬇️
apps/opentelemetry/src/otel_span_ets.erl 59.45% <87.50%> (+0.63%) ⬆️
apps/opentelemetry/src/otel_span_utils.erl 96.55% <93.33%> (+4.88%) ⬆️
apps/opentelemetry/src/otel_batch_processor.erl 72.72% <100.00%> (-0.84%) ⬇️
apps/opentelemetry/src/otel_propagator_http_b3.erl 64.51% <100.00%> (ø)
...pps/opentelemetry/src/otel_propagator_http_w3c.erl 45.45% <100.00%> (ø)
apps/opentelemetry/src/otel_tracer_default.erl 100.00% <100.00%> (+4.54%) ⬆️
apps/opentelemetry_api/src/otel_tracer.erl 58.33% <100.00%> (ø)

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 ee1fc15...54f629e. Read the comment docs.

{SpanCtx, Span=#span{}} ->
%% span isn't recorded so don't run processors
%% but we do insert to ets table?
_ = storage_insert(Span),
Copy link
Member

Choose a reason for hiding this comment

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

Is this a spec thing or just wondering?

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. It is something we need to figure out next and I gave up on completely solving in this PR.

I'm unsure if we need an actual #span{} in the ets table for a recorded but not sampled span.

@tsloughter tsloughter merged commit a3f9fa7 into master Nov 2, 2020
@tsloughter tsloughter deleted the sampler-ctx-drop branch November 2, 2020 15:16
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