Skip to content

Introduce load-balanced channel for OpenTelemetry exporters#2175

Open
dkostyrev wants to merge 3 commits into
TraceMachina:mainfrom
joomcode:feature/balanced-channel
Open

Introduce load-balanced channel for OpenTelemetry exporters#2175
dkostyrev wants to merge 3 commits into
TraceMachina:mainfrom
joomcode:feature/balanced-channel

Conversation

@dkostyrev
Copy link
Copy Markdown
Contributor

@dkostyrev dkostyrev commented Feb 23, 2026 •
edited by MarcusSorealheis
Loading

Summary

This PR introduces client-side load balancing for OpenTelemetry (OTLP) gRPC connections using the ginepro library. When the NL_OTEL_ENDPOINT (name to be discussed, maybe boolean flag?) environment variable is set, NativeLink will create a load-balanced channel for exporting logs, traces, and metrics, distributing requests across multiple backend endpoints resolved via DNS. This change allows to distribute OTLP traffic across multiple OTLP collector instances.

Changes

Load-balanced OTLP exports:

  • Added ginepro dependency to provide client-side load balancing for gRPC channels used by OpenTelemetry exporters
  • Introduced NL_OTEL_ENDPOINT environment variable to configure the OTLP endpoint for load-balanced connections
  • Changed init_tracing() from synchronous to async to support balanced channel initialization
  • Updated OpenTelemetry dependencies from v0.29 to v0.30 to support the new channel configuration
  • All three exporters (logs, traces, metrics) now share the same load-balanced channel when configured

This change is Reviewable

@dkostyrev dkostyrev force-pushed the feature/balanced-channel branch from eb9bfe9 to b86f6d1 Compare February 23, 2026 16:21
@dkostyrev dkostyrev force-pushed the feature/balanced-channel branch from b86f6d1 to 9533feb Compare February 23, 2026 16:27
@dkostyrev dkostyrev force-pushed the feature/balanced-channel branch from 9533feb to 6406230 Compare February 23, 2026 16:35
@MarcusSorealheis MarcusSorealheis requested a review from palfrey March 5, 2026 15:28
@MarcusSorealheis
Copy link
Copy Markdown
Collaborator

MarcusSorealheis commented Mar 5, 2026

@dkostyrev Than you for this awesome PR. I have finally had a bit of time to explore it and now I understand quite a bit.

This is fantastic.

@MarcusSorealheis
Copy link
Copy Markdown
Collaborator

MarcusSorealheis commented Mar 5, 2026

@amankrx when you get a chance, please help with the merge conflict here.

MarcusSorealheis
MarcusSorealheis approved these changes Mar 5, 2026
View reviewed changes
Comment thread src/bin/redis_store_tester.rs
.block_on(async {
// The OTLP exporters need to run in a Tokio context.
spawn!("init tracing", async { init_tracing() })
spawn!("init tracing", async { init_tracing().await })
Copy link
Copy Markdown
Collaborator

@MarcusSorealheis MarcusSorealheis Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oof! Good catch.

@palfrey
Copy link
Copy Markdown
Member

palfrey commented Mar 6, 2026

That coverage issue is interesting. mold: error: undefined symbol: __memcpy_chk is the sort of problem we used to be having before the fixes in #2192 that removes the hardeningDisable bit that was a workaround to this before. Not quite sure why this is hitting here now, but might need to re-add the workaround.

@dkostyrev dkostyrev force-pushed the feature/balanced-channel branch from 29cb3d7 to dff32cd Compare March 6, 2026 12:34
@dkostyrev dkostyrev force-pushed the feature/balanced-channel branch from 31d41d4 to 892ecee Compare March 16, 2026 13:48
@dkostyrev dkostyrev force-pushed the feature/balanced-channel branch from 892ecee to 1eb7f46 Compare March 16, 2026 13:59
@palfrey palfrey mentioned this pull request Mar 30, 2026
Closed
8 tasks
@palfrey
Copy link
Copy Markdown
Member

palfrey commented Mar 30, 2026

I think this is actually hitting https://users.rust-lang.org/t/undefined-reference-to-memcpy-chk-with-target-x86-64-unknown-linux-musl/113567 as it's building a linked zstd, not the vendored one. Upstream has a PR that would sort this out, and I've just prodded them on that.

@MarcusSorealheis
Copy link
Copy Markdown
Collaborator

MarcusSorealheis commented May 8, 2026

@dkostyrev do you want help here?

@dkostyrev dkostyrev force-pushed the feature/balanced-channel branch 2 times, most recently from f322ac7 to b8d6683 Compare May 8, 2026 16:31
@dkostyrev dkostyrev force-pushed the feature/balanced-channel branch from b8d6683 to 2bdfd7c Compare May 8, 2026 16:50
@dkostyrev
Copy link
Copy Markdown
Contributor Author

dkostyrev commented May 8, 2026

@MarcusSorealheis I've reverted opentelemetry update, I think that the coverage should pass now

@palfrey
Copy link
Copy Markdown
Member

palfrey commented May 8, 2026

I managed to break our attic caching. Rebasing on main should fix at least most of these failures

@dkostyrev
Introduce load-balanced channel for OpenTelemetry exporters
ac3d375 
Add client-side load balancing to OTLP gRPC connections using ginepro.
When NL_OTEL_ENDPOINT is set, the telemetry system creates a load-balanced
channel shared across log, trace, and metric exporters. This enables better
distribution of telemetry traffic across multiple OTLP collector instances
and improves overall system resilience.

- Add ginepro dependency for gRPC load balancing
- Upgrade OpenTelemetry dependencies from 0.29 to 0.30
- Change init_tracing() to async to support channel initialization
- Add NL_OTEL_ENDPOINT environment variable for configuration
- Update all OTLP exporters to use shared load-balanced channel

# Conflicts:
#	Cargo.lock
@dkostyrev dkostyrev force-pushed the feature/balanced-channel branch from 2bdfd7c to ac3d375 Compare May 9, 2026 09:32
@dkostyrev
Copy link
Copy Markdown
Contributor Author

dkostyrev commented May 9, 2026

I've rebased the branch, CI is passing now

palfrey
palfrey reviewed May 11, 2026
View reviewed changes
Comment thread nativelink-util/src/telemetry.rs
Copy link
Copy Markdown
Member

@palfrey palfrey May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not seeing any tests for this? Especially if we're introducing an entirely new library that might well do things in a different way to our current stuff (although hopefully identical in the 1-channel case), I'd like to make sure that it's at least about the same.

Copy link
Copy Markdown
Contributor Author

@dkostyrev dkostyrev May 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you proposing adding an integration test for the whole init_tracing function or just test some parts of the new logic in isolation?

Copy link
Copy Markdown
Member

@palfrey palfrey May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer I think an integration test, just to check the new library does opentelemetry in the same way as our older stuff if that's possible?

@dkostyrev dkostyrev force-pushed the feature/balanced-channel branch from bb5068e to d6a38f4 Compare May 14, 2026 14:27
@dkostyrev dkostyrev force-pushed the feature/balanced-channel branch from d6a38f4 to 9e6ddec Compare May 14, 2026 14:34
@dkostyrev @cursoragent
Fix CI failures in telemetry tests
7a99137 
- Replace `let _ = server.await` with `.await.ok()` to satisfy
  `-D let-underscore-drop` on ubuntu/windows stable and coverage builds
- Add `default-features = false` to opentelemetry-proto and
  opentelemetry_sdk dev-dependencies to pass taplo schema validation

Co-authored-by: Cursor <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@palfrey palfrey palfrey left review comments

@MarcusSorealheis MarcusSorealheis MarcusSorealheis approved these changes

@amankrx amankrx amankrx approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dkostyrev @MarcusSorealheis @palfrey @amankrx