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.

Sign up for GitHub

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

Jump to bottom

Support OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT and OTEL_ATTRIBUTE_COUNT_LIMIT #1251

Closed
ahayworth opened this issue May 11, 2022 · 1 comment · Fixed by #1292
Closed

Support OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT and OTEL_ATTRIBUTE_COUNT_LIMIT #1251

ahayworth opened this issue May 11, 2022 · 1 comment · Fixed by #1292
Labels
good first issue Good for newcomers spec-compliance/v1.6.0 spec-compliance Required for OpenTelemetry spec compliance

Comments

@ahayworth
Copy link
Contributor

Added in open-telemetry/opentelemetry-specification#1130

We were early adopters of some of these limits, but they are not formalized in the spec and we should update.
@ahayworth ahayworth added spec-compliance Required for OpenTelemetry spec compliance spec-compliance/v1.6.0 labels May 11, 2022
@fbogsany
Copy link
Contributor

We have the trace-specific versions of these (OTEL_SPAN_ATTRIBUTE_...), and they appear to be correct, according to the spec (the spec calls them "model-specific limits"). We just need to support the more general env vars (OTEL_ATTRIBUTE_...) as fallback options here:

opentelemetry-ruby/sdk/lib/opentelemetry/sdk/trace/span_limits.rb

Lines 34 to 40 in 113e513

def initialize(attribute_count_limit: Integer(ENV.fetch('OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT', 128)), # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
attribute_length_limit: ENV.fetch('OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT',
ENV['OTEL_RUBY_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT']),
event_count_limit: Integer(ENV.fetch('OTEL_SPAN_EVENT_COUNT_LIMIT', 128)),
link_count_limit: Integer(ENV.fetch('OTEL_SPAN_LINK_COUNT_LIMIT', 128)),
event_attribute_count_limit: Integer(ENV.fetch('OTEL_EVENT_ATTRIBUTE_COUNT_LIMIT', 128)),
link_attribute_count_limit: Integer(ENV.fetch('OTEL_LINK_ATTRIBUTE_COUNT_LIMIT', 128)))

@plantfansam plantfansam added the good first issue Good for newcomers label May 25, 2022
ahayworth added a commit to ahayworth/opentelemetry-ruby that referenced this issue Jun 4, 2022
@ahayworth
feat: support OTEL_ATTRIBUTE_{COUNT,VALUE_LENGTH}_LIMIT env vars
1829611 
Fixes open-telemetry#1251 by additionally supporting `OTEL_ATTRIBUTE_COUNT_LIMIT` and
`OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT` as ways to configure span limits. As
per spec, the existing model-specific limits (eg: `OTEL_SPAN_ATTRIBUTE_...`
env vars) are preferred when they are present.

The easiest way to do this was to re-use the handy `config_opt` method
in `opentelemetry-common` package, on which we already depend. It made
the lines even longer, which is gross, but it works pretty nicely.
@ahayworth ahayworth mentioned this issue Jun 4, 2022
Merged
fbogsany added a commit that referenced this issue Jun 21, 2022
@ahayworth @fbogsany
feat: support OTEL_ATTRIBUTE_{COUNT,VALUE_LENGTH}_LIMIT env vars (#1292)
e9bd2b2 
Fixes #1251 by additionally supporting `OTEL_ATTRIBUTE_COUNT_LIMIT` and
`OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT` as ways to configure span limits. As
per spec, the existing model-specific limits (eg: `OTEL_SPAN_ATTRIBUTE_...`
env vars) are preferred when they are present.

The easiest way to do this was to re-use the handy `config_opt` method
in `opentelemetry-common` package, on which we already depend. It made
the lines even longer, which is gross, but it works pretty nicely.

Co-authored-by: Francis Bogsanyi <francis.bogsanyi@shopify.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Good for newcomers spec-compliance/v1.6.0 spec-compliance Required for OpenTelemetry spec compliance
Projects
Ruby - Spec Compliance
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: support OTEL_ATTRIBUTE_{COUNT,VALUE_LENGTH}_LIMIT env vars ahayworth/opentelemetry-ruby
3 participants
@plantfansam @fbogsany @ahayworth