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

Span attribute count limit doesn't work by the specification #2671

Closed
Bataran opened this issue Dec 16, 2021 · 1 comment · Fixed by #2678
Closed

Span attribute count limit doesn't work by the specification #2671

Bataran opened this issue Dec 16, 2021 · 1 comment · Fixed by #2678
Assignees
Labels
bug Something isn't working

Comments

@Bataran
Copy link
Contributor

Bataran commented Dec 16, 2021

Please answer these questions before submitting a bug report.

What version of OpenTelemetry are you using?

"@opentelemetry/api": "^1.0.3",
"@opentelemetry/sdk-trace-base": "^1.0.1"

What version of Node are you using?

v16.13.0

Please provide the code you used to setup the OpenTelemetry SDK

// index.js
const opentelemetry = require('@opentelemetry/api');
const { BasicTracerProvider } = require('@opentelemetry/sdk-trace-base');

new BasicTracerProvider().register();

const span = opentelemetry.trace.getTracer('default').startSpan('foo');

span.setAttribute('key', 'value');
span.setAttribute('foo', 'bar');
span.setAttribute('mirko', 'slavko');
// the span has 1 attribute even though it's been run with OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT=128 and OTEL_ATTRIBUTE_COUNT_LIMIT=1

span.end();

What did you do?

I started the script with OTEL_ATTRIBUTE_COUNT_LIMIT set to 1 and OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT set to 128
env OTEL_ATTRIBUTE_COUNT_LIMIT=1 OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT=128 node index.js

What did you expect to see?

I expected it works by the specification:

If both a general and a model-specific limit are implemented, then the SDK MUST first attempt to use the model-specific limit, if it isn't set, then the SDK MUST attempt to use the general limit

What did you see instead?

The general limit is used instead the model-specific. So the limit is set to 1 not 128

Additional context

Add any other context about the problem here.

@Bataran Bataran added the bug Something isn't working label Dec 16, 2021
@Bataran
Copy link
Contributor Author

Bataran commented Dec 16, 2021

Hi @dyladan, could you assign this to me? I'm already working on it

Bataran added a commit to Bataran/opentelemetry-js that referenced this issue Dec 17, 2021
Bataran added a commit to Bataran/opentelemetry-js that referenced this issue Dec 20, 2021
Bataran added a commit to Bataran/opentelemetry-js that referenced this issue Dec 21, 2021
Bataran added a commit to Bataran/opentelemetry-js that referenced this issue Dec 21, 2021
rauno56 added a commit to Bataran/opentelemetry-js that referenced this issue Jan 5, 2022
dyladan added a commit to Bataran/opentelemetry-js that referenced this issue Jan 5, 2022
vmarchaud added a commit to Bataran/opentelemetry-js that referenced this issue Jan 8, 2022
legendecas added a commit to Bataran/opentelemetry-js that referenced this issue Jan 11, 2022
Bataran added a commit to Bataran/opentelemetry-js that referenced this issue Jan 12, 2022
legendecas added a commit that referenced this issue Jan 12, 2022
Co-authored-by: Rauno Viskus <Rauno56@users.noreply.github.com>
Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com>
Co-authored-by: Valentin Marchaud <contact@vmarchaud.fr>
Co-authored-by: legendecas <legendecas@gmail.com>
rauno56 added a commit to rauno56/opentelemetry-js that referenced this issue Jan 14, 2022
…n-telemetry#2678)


Co-authored-by: Rauno Viskus <Rauno56@users.noreply.github.com>
Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com>
Co-authored-by: Valentin Marchaud <contact@vmarchaud.fr>
Co-authored-by: legendecas <legendecas@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant