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

Configuration improvements and span limit settings #327

Merged
merged 5 commits into from
Dec 13, 2021

Conversation

tsloughter
Copy link
Member

For span limits this only has the record and sets it to a persistent term through the application environment or OS environment configuration.

Once I realized I wanted to improve the configuration I stopped at that point to open a PR and then will follow it up with one that actually utilizes the span limits.

Two main issues were resolved with the configuration updates here: first, it will now fall back to the default value if transforming the user supplied value fails, and second, application environment configuration values are transformed as well, so setting a string for a limit in the application environment will fallback to the default as well.

Tristan Sloughter added 3 commits December 6, 2021 16:05
these changes ensure all configuration goes through transform
checks to verify their type, as best we can. Before only the
values from OS environment variables would be transformed. This
could lead to configuration values like "string" for a key
like attribute_count_limit that requires an integer.
@codecov
Copy link

codecov bot commented Dec 10, 2021

Codecov Report

Merging #327 (68f0341) into main (20c89cf) will increase coverage by 0.01%.
The diff coverage is 82.29%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #327      +/-   ##
==========================================
+ Coverage   38.47%   38.49%   +0.01%     
==========================================
  Files          50       51       +1     
  Lines        3462     3510      +48     
==========================================
+ Hits         1332     1351      +19     
- Misses       2130     2159      +29     
Flag Coverage Δ
api 64.39% <ø> (ø)
elixir 15.12% <ø> (ø)
erlang 38.51% <82.29%> (+0.01%) ⬆️
exporter 21.68% <100.00%> (-0.10%) ⬇️
sdk 74.01% <81.52%> (-2.17%) ⬇️

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

Impacted Files Coverage Δ
apps/opentelemetry/src/otel_configuration.erl 74.77% <75.36%> (-5.79%) ⬇️
apps/opentelemetry/src/opentelemetry_app.erl 100.00% <100.00%> (ø)
apps/opentelemetry/src/opentelemetry_sup.erl 100.00% <100.00%> (ø)
apps/opentelemetry/src/otel_resource_detector.erl 83.78% <100.00%> (-8.11%) ⬇️
apps/opentelemetry/src/otel_span_limits.erl 100.00% <100.00%> (ø)
apps/opentelemetry/src/otel_span_sup.erl 87.50% <100.00%> (ø)
apps/opentelemetry/src/otel_tracer_server.erl 72.72% <100.00%> (ø)
...ntelemetry_exporter/src/opentelemetry_exporter.erl 72.54% <100.00%> (-1.80%) ⬇️
apps/opentelemetry/src/otel_exporter.erl 21.42% <0.00%> (-21.43%) ⬇️
... and 2 more

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 20c89cf...68f0341. Read the comment docs.

@tsloughter tsloughter merged commit 70681ed into open-telemetry:main Dec 13, 2021
@tsloughter tsloughter deleted the span-limits branch December 13, 2021 13:21
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