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

make trace/span id generation customizable #310

Merged
merged 3 commits into from
Nov 18, 2021

Conversation

tsloughter
Copy link
Member

A behaviour, otel_id_generator, is added which has callbacks for
generating trace and span ids, along with the default versions
of those functions.

The implementation to use is kept in the tracer record.

@tsloughter
Copy link
Member Author

Crap, just realized this is wrong in the noop tracer.

@codecov
Copy link

codecov bot commented Nov 12, 2021

Codecov Report

Merging #310 (fa8ae84) into main (590d87f) will decrease coverage by 0.06%.
The diff coverage is 78.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #310      +/-   ##
==========================================
- Coverage   37.90%   37.84%   -0.07%     
==========================================
  Files          49       50       +1     
  Lines        3327     3327              
==========================================
- Hits         1261     1259       -2     
- Misses       2066     2068       +2     
Flag Coverage Δ
api 64.82% <100.00%> (-0.33%) ⬇️
elixir 14.73% <0.00%> (+0.13%) ⬆️
erlang 37.85% <78.57%> (-0.07%) ⬇️
exporter 19.75% <ø> (ø)
sdk 75.70% <76.92%> (-0.13%) ⬇️

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

Impacted Files Coverage Δ
apps/opentelemetry_api/src/opentelemetry.erl 75.32% <ø> (-0.93%) ⬇️
apps/opentelemetry/src/otel_id_generator.erl 50.00% <50.00%> (ø)
apps/opentelemetry/src/otel_span_utils.erl 88.00% <83.33%> (ø)
apps/opentelemetry/src/otel_span_ets.erl 64.86% <100.00%> (ø)
apps/opentelemetry/src/otel_tracer_default.erl 100.00% <100.00%> (ø)
apps/opentelemetry/src/otel_tracer_server.erl 75.00% <100.00%> (+0.58%) ⬆️
apps/opentelemetry_api/src/otel_tracer_noop.erl 56.25% <100.00%> (-4.87%) ⬇️

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 590d87f...fa8ae84. Read the comment docs.

A behaviour, otel_id_generator, is added which has callbacks for
generating trace and span ids, along with the default versions
of those functions.

The implementation to use is kept in the tracer record.
@tsloughter
Copy link
Member Author

Fixed, based on my PR to the spec, will have to see how it goes open-telemetry/opentelemetry-specification#2121

otel_span:start_opts(), fun(), otel_tracer_server:instrumentation_library())
-> opentelemetry:span_ctx().
start_span(Ctx, Name, Sampler, IdGeneratorModule, Opts, Processors, InstrumentationLibrary) ->
case otel_span_utils:start_span(Ctx, Name, Sampler, IdGeneratorModule, Opts) of
Copy link
Member

Choose a reason for hiding this comment

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

starting to think this whole injection mechanism coming from other langs means we'd eventually be better served by a default map that we merge with user options to override individual entries.

If everything in the universe can be overridden, the number of call params will never end growing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Eh, it is a little different. It isn't an override the user is passing, it is simply that more has been added to the Tracer and before there was no need to pass the actual Tracer down because there were just a couple pieces needed from it.

It may be that we should be passing the actual Tracer all the way down and not pulling out fields. I may make a PR to do that. I think pretty much everything in the Tracer is now an argument.

@tsloughter tsloughter merged commit ae88d88 into open-telemetry:main Nov 18, 2021
@tsloughter tsloughter deleted the id-generators branch November 18, 2021 21:55
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