Add OpenTelemetry support - #615
Draft
edolstra wants to merge 29 commits into
Draft
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueComment |
Add a vendor-neutral distributed tracing facade (nix::otel) as the foundation for tracing binary cache requests from the Nix client through the daemon to FlakeHub Cache. The facade (nix/util/otel.hh) is flag-independent: no conditionals or OpenTelemetry includes in the installed header. The implementation is gated behind a new 'otel' meson feature option; when disabled (or when enabled but OTEL_EXPORTER_OTLP_ENDPOINT is not set), every operation is a cheap no-op. Design notes: * Spans are copyable shared-handle values, safe to move across threads and to end out of stack order, so they can later back FileTransfer requests (which complete on the curl worker thread) and Activities. * Span::injectContext() returns W3C traceparent/tracestate headers for propagation to HTTP servers and the daemon; startSpanFromRemoteParent() continues a trace on the receiving side. * The tracer provider is owned by a deliberately leaked global instead of opentelemetry's Provider singleton so that no exporter teardown can run from static destructors at exit() time (cf. the OPENSSL_INIT_NO_ATEXIT note in util.cc); flushing happens only via an explicit forceFlushAndShutdown() with bounded timeouts. * Only the OTLP/HTTP exporter is used (not gRPC, which is heavy and fork-unsafe). nixpkgs' opentelemetry-cpp ships no OTLP exporter by default, so packaging/dependencies.nix overrides it with enableHttp = true. Nothing calls init() yet; wiring into the CLI, the daemon, and FileTransfer comes separately. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Wire the nix::otel facade into the CLI entry point:
* otel::init() runs after programName is resolved and before the
legacy command dispatch, so nix, nix-daemon, build-remote etc. all
get a provider with the appropriate service.name. It remains a
no-op unless OTEL_EXPORTER_OTLP_ENDPOINT (or
OTEL_EXPORTER_OTLP_TRACES_ENDPOINT) is set in the environment.
* A Finally flushes and shuts down the exporter on every exit path,
including exceptions and legacy commands' early returns.
* Each command invocation gets a root span ("nix <subcommand>"),
ended by RAII. On failure the span records an error status with the
ANSI-stripped error message; Exit is rethrown unmarked since it can
represent a clean exit.
Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Give every FileTransfer a span, parented under the process root span and exported as a CLIENT span covering the whole transfer including retries. On success it records http.response.status_code (and http.request.resend_count when retries happened); every failure path funnels through failEx(), which records an error status with the ANSI-stripped message. The span's W3C context (traceparent/tracestate) is injected into the request headers, so any otel-instrumented server (e.g. FlakeHub Cache) can join its server-side spans to the client's trace. To parent transfer spans correctly, the facade gains otel::setRootSpan() / otel::rootSpan(): a process-wide default parent registered by main.cc. Only a weak reference is kept so the root span's RAII lifetime is unaffected. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Split the FileTransfer span into two levels: a span covering the lifetime of the TransferItem (including retries and backoff), and a child CLIENT span per HTTP request, recreated on every attempt. Each attempt span records its own http.response.status_code, its resend count, and its own error status when that attempt fails -- even if the transfer later succeeds on a retry. Since the traceparent now differs per attempt, the request headers are rebuilt in init() rather than once in the constructor, so every HTTP request propagates the context of exactly the attempt that produced it. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Every caller wants this (error messages are formatted for the terminal), so do it in the facade instead of at each call site. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Add an "open-telemetry" worker protocol feature. When both sides support it, the client sends the W3C traceparent of its root span (or an empty string if it is not tracing) right after the handshake in initConnection(). The daemon then opens a "daemon connection" SERVER span parented under the client's trace and registers it as its root span, so everything it does on behalf of the connection -- including binary cache requests once they happen daemon-side -- joins the client's trace. Since the daemon forks a child per connection and the OpenTelemetry batch exporter's worker thread does not survive fork(), the facade gains otel::resetAfterFork(), which discards the inherited provider without touching it. The connection child calls it, initializes tracing afresh as "nix-daemon", and flushes before exit. Feature negotiation keeps old clients and daemons compatible in both directions: without agreement, no traceparent is exchanged. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Build opentelemetry-cpp with WITH_OTLP_HTTP_COMPRESSION=ON (and zlib), so that OTEL_EXPORTER_OTLP_COMPRESSION=gzip works instead of warning and falling back to uncompressed exports. The flag also enables two gzip HTTP round-trip tests that time out flakily in the build sandbox (their sibling BasicCurlHttpTests are already shipped disabled), so exclude them from the check phase. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Instead of creating spans explicitly at instrumented call sites,
derive them from Nix's existing Activity system: an
OpenTelemetryLogger (analogous to JSONLogger) maps activities onto
spans and is added to the global logger via the new
applyExtraLogger(). All OpenTelemetry code moves from libutil to
src/nix (otel-logger.{cc,hh}), along with the build-time dependency;
libutil and libstore are now free of it. Anything that emits an
Activity gets traced for free, which is also how future
instrumentation (e.g. the evaluator) should hook in.
Vendor-neutral additions to libutil's logging interface:
* Logger::getTraceContext(): returns W3C trace context headers for an
activity, used by FileTransfer to add a traceparent header to HTTP
requests and by RemoteStore to propagate the trace context to the
daemon during the handshake (the "open-telemetry" protocol feature
is unchanged on the wire).
* RemoteLogSource: marks Logger calls that replay messages from
another process. The client-side handling of activities forwarded
from the daemon uses it, so that the client does not export
telemetry for them.
* actFileTransferAttempt / resHttpStatus: FileTransfer now starts a
child activity per HTTP request (there can be several per transfer
due to retries) and reports the response status on it. The transfer
activity is started eagerly in init() so that it exists before the
request headers are built.
Export ownership: every process exports exactly the activities it
originates. The daemon keeps its own exporter lifecycle; its
connection span (created via processConnection()'s new setupTelemetry
callback) is parented under the client's trace context, and its
activities are exported by the daemon itself while still being
forwarded to the client for display. The client skips forwarded
activities, so nothing is exported twice.
Known limitations: span error status is mostly lossy (stopActivity
carries no status; the root span still records command failures), and
top-level daemon activities parent under the connection span rather
than the client-side operation that triggered them.
Assisted-by: Claude Fable 5 <noreply@anthropic.com>
edolstra
force-pushed
the
eelcodolstra/nix-467
branch
from
September 3, 2026 11:05
77e6526 to
cf65665
Compare
Turn the global logger into a TeeLogger if it isn't one already, then unconditionally append the extra logger to it. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
It no longer has any callers: applyExtraLogger() constructs the TeeLogger directly. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
The JSON payload now carries `method` (e.g. `GET`) and `bodySize` (the number of body bytes received) alongside `httpStatus`. The OpenTelemetryLogger maps them onto the `http.request.method` and `http.response.body.size` span attributes. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Per the OpenTelemetry semantic conventions, HTTP client spans are named after the request method (e.g. `GET`). Pass the method as a field of actFileTransferAttempt so the OpenTelemetryLogger can use it as the span name and the `http.request.method` attribute, and drop it from resHttpStatus where it is now superfluous. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Return the name of the enum value without the `act` prefix (e.g. `OptimiseStore`), generated with a macro so the names cannot drift from the enum. In the future, C++26 reflection can make this fully generic. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
ActivityType is an enum, so adding a new type of activity is expensive (it requires changing logging.hh) and not self-describing. Also, activity fields are positional, with their meaning living in per-type conventions. So add a new Activity constructor that takes a string activity name (e.g. `FileTransferAttempt`) and key/value meta-information about the activity. Metadata keys follow the OpenTelemetry attribute naming conventions where applicable (e.g. `http.request.method`); Nix-specific keys use a `nix.` prefix. This is backwards compatible: the default implementation of the new name-based Logger::startActivity() reports the activity through the old ActivityType-based interface as the new type `actStringly`, discarding the name and metadata. So legacy loggers (including the daemon's TunnelLogger, and therefore old clients) work unchanged. Loggers that know about the new interface get the rich form: JSONLogger emits the name and a `payload` object with the metadata, and the OpenTelemetryLogger uses the name as the span name and the metadata as span attributes, without needing per-type knowledge. FileTransfer's per-HTTP-request activity becomes the first string-named activity, replacing the actFileTransferAttempt enum value introduced recently. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Rename the anonymous "evaluating derivation" activity in InstallableFlake::toDerivedPaths() to `EvaluateFlakeDerivationOutput`, carrying the installable as metadata. This makes flake evaluation show up as a properly named span in OpenTelemetry traces. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Handy for finding the trace of a particular Nix invocation in the tracing backend. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
The anonymous activity in fetchToStore2() becomes `CopySourcePath` or `HashSourcePath` (depending on the fetch mode), carrying the source path as metadata. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Also register it as the current activity (via PushActivity), so that the QueryPathInfo activities triggered by it are parented under it instead of dangling from the root. (The asio coroutines involved all run on the calling thread via ctx.run(), so the thread-local current activity is visible inside them; only the push was missing.) Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Parent the worker's aggregate activities (actBuilds, actCopyPaths) under its actRealise activity, parent actSubstitute under actCopyPaths, and parent the actBuild activities (both the local build and build hook cases) under actBuilds. The actSubstitute case requires passing the parent activity ID into the substitution thread explicitly, since the current activity is thread-local and the substitution runs on a fresh thread. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Previously all daemon activity was parented under a single "daemon connection" span. Instead, the opcode send is now factored into WorkerProto::BasicClientConnection::startOp(), which appends the trace context of the client's current activity when the "open-telemetry" protocol feature has been negotiated (safe to redefine since it was never released). On the daemon side, the traceparent is read centrally in the processConnection() loop, which wraps each operation in a generic "daemon operation" activity carrying the numeric opcode as `nix.daemon.op` metadata and the received trace context as `traceparent` metadata. The OpenTelemetryLogger treats such a metadata field as a remote parent, taking precedence over the local parent. Everything the daemon does on behalf of an operation then nests under the client's span via the current-activity mechanism. Note that in the daemon, the per-operation activity must be created on the global logger rather than the TunnelLogger: sending it only to the TunnelLogger would forward it to the client (which ignores remote activities) without ever reaching the daemon's own OpenTelemetryLogger. Also add getTraceparent() to extract the traceparent header from Logger::getTraceContext() results. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
A typical Nix invocation performs thousands of daemon operations, so emitting a span per "daemon operation" activity is very spammy. Instead, an activity carrying a `traceparent` metadata field now produces no span at all: the OpenTelemetryLogger records the remote context (as a non-recording DefaultSpan) so that child activities resolve their parent to the client's span, linking the daemon's work directly under the client activity that initiated the operation. Also, the daemon only creates the link activity when the client actually sent a trace context. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Print the trace ID at the end, in flushOtelAndShutdown(), so that when the user sees it, the trace has actually been uploaded to the collector. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Add a string-named activity covering the lifetime of a PathSubstitutionGoal, starting as soon as init() runs. The substituter queryPathInfo() activities and the actSubstitute activity (which is left alone, since the progress bar uses it to indicate that the actual download is happening) are now parented under it, instead of dangling from actCopyPaths or the root. Note that the PushActivity around queryPathInfo() is narrowly scoped to the synchronous part of the call, so that it does not extend across a coroutine suspension point (the thread-local current activity would leak into unrelated interleaved coroutines). Assisted-by: Claude Fable 5 <noreply@anthropic.com>
edolstra
force-pushed
the
eelcodolstra/nix-467
branch
from
September 3, 2026 18:43
bcddde6 to
b3cab19
Compare
stopActivity() carries no success/failure status, so spans generally ended without one. However, if an Activity is destroyed by stack unwinding (std::uncaught_exceptions() is non-zero in stopActivity(), which runs on the unwinding thread), we can infer that the activity failed and set an error status on its span. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Honor the standard OTEL_TRACES_SAMPLER / OTEL_TRACES_SAMPLER_ARG environment variables (which the C++ SDK does not read itself), supporting the sampler names from the OpenTelemetry specification: always_on, always_off, traceidratio and their parentbased_* variants. The default remains parentbased_always_on, i.e. record everything. The parentbased_* samplers follow the sampling decision of the parent span, which propagates in the sampled flag of the W3C trace context — so e.g. the daemon and binary cache servers follow the client's decision, keeping traces complete or absent as a whole. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Assisted-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Context