tests: prevent port binding failures in concurrent OTEL tests#11808
Conversation
Signed-off-by: Patrick Stephens <pat@telemetryforge.io>
📝 WalkthroughWalkthroughPort number for the OpenTelemetry test input is updated from 4318 to 4319 in both the C test macro constant and the corresponding YAML test configuration, with added comments documenting the synchronization requirement between files. ChangesOpenTelemetry test port update
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/runtime/in_opentelemetry_routing.c (1)
41-43: ⚡ Quick winRemove the unused
PORT_OTELmacro or use it programmatically to sync with the YAML configuration.The
PORT_OTELmacro is defined here but not used anywhere in this test file. The actual port binding comes from the YAML configuration (referenced at line 246), making this macro a documentation artifact. The comments acknowledge the need to keep it in sync withdata/routing/otlp_comprehensive_routing_test.yaml, but this manual synchronization is error-prone.Either remove the unused macro entirely or refactor to read the port from YAML programmatically to eliminate the synchronization burden.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/runtime/in_opentelemetry_routing.c` around lines 41 - 43, The PORT_OTEL macro is defined but unused; either remove the macro and its sync comment entirely or replace it with a runtime value read from the YAML so the test can't drift from data/routing/otlp_comprehensive_routing_test.yaml. If removing: delete the PORT_OTEL `#define` and the "pick a port" comment and ensure no references remain. If keeping: add code in the test setup to parse data/routing/otlp_comprehensive_routing_test.yaml (the YAML referenced at line ~246) to extract the OTLP port and assign it to a local variable (use that variable where the test binds/validates the port instead of the macro PORT_OTEL). Ensure all occurrences of PORT_OTEL in this test are replaced by the runtime variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/runtime/in_opentelemetry_routing.c`:
- Around line 41-43: The PORT_OTEL macro is defined but unused; either remove
the macro and its sync comment entirely or replace it with a runtime value read
from the YAML so the test can't drift from
data/routing/otlp_comprehensive_routing_test.yaml. If removing: delete the
PORT_OTEL `#define` and the "pick a port" comment and ensure no references remain.
If keeping: add code in the test setup to parse
data/routing/otlp_comprehensive_routing_test.yaml (the YAML referenced at line
~246) to extract the OTLP port and assign it to a local variable (use that
variable where the test binds/validates the port instead of the macro
PORT_OTEL). Ensure all occurrences of PORT_OTEL in this test are replaced by the
runtime variable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1f2e7d37-5214-4cfe-a526-4375099ab2e7
📒 Files selected for processing (2)
tests/runtime/data/routing/otlp_comprehensive_routing_test.yamltests/runtime/in_opentelemetry_routing.c
|
Looks like the tail input tests are failing but that is unrelated to these changes and is happening upstream as well: https://github.com/fluent/fluent-bit/actions/runs/25892842380/job/76099561690 |
edsiper
deleted the
prevent_concurrent_test_failures_flb-rt-in_opentelemtry
branch
|
thanks! (yep, taking a look at the rt-tail one) |
2 participants
When tests are run concurrently, the
flb-rt-in_opentelemetryandflb-rt-in_opentelemetry_routingtests both try to bind to the same port so tweaked the test configuration to use different ports.Discovered during downstream testing where we happen to run both tests at the same time which does not seem to happen here (but purely by chance): https://github.com/telemetryforge/agent/actions/runs/25921061604/job/76189906835?pr=277
Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.