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

(otelarrowreceiver): Fix incorrect OTLP admission control behavior #36078

Merged
merged 4 commits into from
Oct 30, 2024

Conversation

jmacd
Copy link
Contributor

@jmacd jmacd commented Oct 29, 2024

Description

#36074 describes several defects we have discovered in the OTel-Arrow admission control logic. This fixes the most urgent defect, which is an early return from the OTLP handlers when admission control fails. In this case, the telemetry signal would disappear because the code returns before calling the obsrecv object.

Link to tracking issue

Part of #36074

Testing

The three signals have three identical tests.

One new test (three times) ensures that spans are still emitted when admission control fails.

Documentation

No functional change.

@djaglowski djaglowski merged commit 2e60343 into open-telemetry:main Oct 30, 2024
158 checks passed
@github-actions github-actions bot added this to the next release milestone Oct 30, 2024
djaglowski pushed a commit that referenced this pull request Nov 1, 2024
#### Description

Simplifies the admission control logic for OTAP payloads. We call
Acquire() once after uncompressing the data, instead of once with
compressed size and once with the difference.

#### Link to tracking issue

Part of #36074.

#### Testing

One test is replaced with logic to verify certain BoundedQueue actions.

~Note: the OTel-Arrow test suite will not pass with this PR until it
merges with #36078.~ Originally developed in #36033.

#### Documentation

Not user-visible.
ArthurSens pushed a commit to ArthurSens/opentelemetry-collector-contrib that referenced this pull request Nov 4, 2024
ArthurSens pushed a commit to ArthurSens/opentelemetry-collector-contrib that referenced this pull request Nov 4, 2024
…telemetry#36082)

#### Description

Simplifies the admission control logic for OTAP payloads. We call
Acquire() once after uncompressing the data, instead of once with
compressed size and once with the difference.

#### Link to tracking issue

Part of open-telemetry#36074.

#### Testing

One test is replaced with logic to verify certain BoundedQueue actions.

~Note: the OTel-Arrow test suite will not pass with this PR until it
merges with open-telemetry#36078.~ Originally developed in open-telemetry#36033.

#### Documentation

Not user-visible.
sbylica-splunk pushed a commit to sbylica-splunk/opentelemetry-collector-contrib that referenced this pull request Dec 17, 2024
sbylica-splunk pushed a commit to sbylica-splunk/opentelemetry-collector-contrib that referenced this pull request Dec 17, 2024
…telemetry#36082)

#### Description

Simplifies the admission control logic for OTAP payloads. We call
Acquire() once after uncompressing the data, instead of once with
compressed size and once with the difference.

#### Link to tracking issue

Part of open-telemetry#36074.

#### Testing

One test is replaced with logic to verify certain BoundedQueue actions.

~Note: the OTel-Arrow test suite will not pass with this PR until it
merges with open-telemetry#36078.~ Originally developed in open-telemetry#36033.

#### Documentation

Not user-visible.
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.

5 participants