initial tracing and log ingestion setup - #260
Merged
bennyz merged 1 commit intoApr 27, 2026
Merged
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (201)
📒 Files selected for processing (23)
📝 WalkthroughWalkthroughAdds OpenTelemetry tracing across operator and build API, propagates trace IDs into Tekton tasks and resources, introduces observability label/annotation constants and sanitization helper, extends OperatorConfig with tracing fields, updates APIs/OpenAPI, adds tests, docs, and dependency bumps. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,220,255,0.5)
OperatorStartup->>KubeAPI: Get OperatorConfig (tracing settings)
end
rect rgba(200,255,200,0.5)
OperatorStartup->>Telemetry: InitTracing(endpoint, sampling, insecure)
Telemetry->>OTLPCollector: Establish OTLP gRPC exporter
end
rect rgba(255,230,200,0.5)
Operator->>KubeAPI: Reconcile ImageBuild / Workspace
Operator->>Tekton: Create PipelineRun/TaskRun (include trace-id param & labels)
Tekton->>TaskPods: Start containers with ADO_TRACE_ID env
TaskPods->>OTLPCollector: Export spans/traces (OTLP)
OTLPCollector->>TracingBackend: Store/visualize traces (Tempo)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
bennyz
force-pushed
the
worktree-monitoring-config
branch
from
April 26, 2026 17:57
6e7614f to
0cd0d4c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (7)
internal/controller/workspace/controller.go (1)
64-138: Spans don't record errors — failures will look like successful spans.
Reconcile,ensurePVC, andensurePodstart spans but never callspan.RecordError(err)/span.SetStatus(codes.Error, ...)on their error paths. With the current code, an error reconciliation produces a span identical to a successful one in the tracing backend, which defeats much of the observability value.Sketch
import ( "go.opentelemetry.io/otel/codes" ) defer func() { if retErr != nil { span.RecordError(retErr) span.SetStatus(codes.Error, retErr.Error()) } span.End() }()Apply to
Reconcile,ensurePVC,ensurePod(use named returns to captureretErr).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/controller/workspace/controller.go` around lines 64 - 138, The spans created in Reconcile (wsTracer.Start) and in ensurePVC and ensurePod do not record errors or set span status on failure; change each function to use a named error return (e.g., retErr error) and replace the simple defer span.End() with a defer that checks retErr and, if non-nil, calls span.RecordError(retErr) and span.SetStatus(codes.Error, retErr.Error()) before ending the span (import go.opentelemetry.io/otel/codes). Apply this pattern to the functions named Reconcile, ensurePVC, and ensurePod so tracing reflects actual error outcomes.internal/common/tasks/tasks.go (1)
1505-1572: Consider also threadingtrace-idthroughprepare-builderand sealed tasks for end-to-end correlation.
GeneratePrepareBuilderTaskand the sealed task spec (sealedTaskSpec) don't take atrace-idparameter or setADO_TRACE_ID, so logs from those task containers won't be correlatable with the parent trace. If they're meant to participate in the same logical build trace, add thetraceIDParamSpec()/traceIDEnvVar()here too; otherwise a code comment explaining why they're excluded would help.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/common/tasks/tasks.go` around lines 1505 - 1572, GeneratePrepareBuilderTask and the sealedTaskSpec are missing the trace-id param/env so their containers don't get ADO_TRACE_ID for log correlation; add traceIDParamSpec() to the TaskSpec Params and include traceIDEnvVar() in the task container/env definitions (e.g., where Params are declared in GeneratePrepareBuilderTask and in sealedTaskSpec container/env setup) so the generated Tekton Task accepts a trace-id and sets ADO_TRACE_ID for all task steps; if you intentionally exclude tracing, add a brief comment in GeneratePrepareBuilderTask and sealedTaskSpec explaining why the trace-id is omitted.cmd/main.go (1)
117-126:defer shutdownTracingis bypassed by all subsequentos.Exit(1)paths.Go does not run deferred functions when
os.Exitis called. The mode-validation block immediately below, plus everysetupLog.Error(...); os.Exit(1)site through the rest ofmain, will skip tracing shutdown — meaning any spans buffered by the OTLP batcher are dropped on startup failures. For startup-time failures the loss is minor, but if you'd like the shutdown to run consistently, factormaininto arun() errorhelper and exit from there:Refactor sketch
-func main() { +func main() { + if err := run(); err != nil { + setupLog.Error(err, "fatal") + os.Exit(1) + } +} + +func run() error { ... - shutdownTracing, err := telemetry.InitTracing(context.Background()) + shutdownTracing, err := telemetry.InitTracing(context.Background()) ... - defer func() { + defer func() { if err := shutdownTracing(context.Background()); err != nil { setupLog.Error(err, "failed to shutdown tracing") } }() ... - if mode != modeAll && mode != modePlatform && mode != modeBuild { - setupLog.Error(fmt.Errorf("invalid mode %q", mode), "...") - os.Exit(1) - } + if mode != modeAll && mode != modePlatform && mode != modeBuild { + return fmt.Errorf("invalid mode %q", mode) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/main.go` around lines 117 - 126, The deferred shutdownTracing call is bypassed by any os.Exit path in main; refactor main by extracting the startup and runtime logic into a run() error function (e.g., move mode validation and the rest of main's logic into run), let main call run() and capture its error, and perform shutdownTracing in main after run returns (or in a deferred in main that runs because you no longer call os.Exit from inside run); ensure all early-return errors from run return an error instead of calling os.Exit directly so shutdownTracing(shutdownCtx) is always executed before exiting.internal/common/telemetry/tracing_test.go (1)
36-46: Consider eliminating the livelocalhost:4317connection attempt.The OTLP gRPC exporter starts a background connection to
http://localhost:4317. In sandboxed CI environments without a collector, this is harmless (connection failures are async) but it does spawn a goroutine that runs untilShutdownreturns, and theShutdowncall here will block on the gRPC dial timeout, slowing the test. If you want a deterministic + fast test, point at anhttptest-style local listener or use a no-op exporter via a test seam inInitTracing(e.g., factor exporter creation behind an interface).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/common/telemetry/tracing_test.go` around lines 36 - 46, The test currently forces a live OTLP gRPC connection by setting OTEL_EXPORTER_OTLP_ENDPOINT to http://localhost:4317 which causes InitTracing (and its returned shutdown) to block on gRPC dial timeouts; change the test to avoid a real dial by either (A) creating a local in-process test collector (e.g., httptest.Listener/Server) and point OTEL_EXPORTER_OTLP_ENDPOINT to that, or (B) add a test seam to InitTracing to accept an exporter factory or a flag to install a no-op/test exporter, and call InitTracing with that in the test; update the test to use the new seam (or local server) so shutdown returns quickly and no background gRPC dial is attempted (referencing InitTracing, shutdown and OTEL_EXPORTER_OTLP_ENDPOINT).internal/common/telemetry/tracing.go (1)
36-40:ParentBased(AlwaysSample)will export every controller span.For an operator with hot reconcile loops (this one wraps
Reconcileplus 6+ phase handlers per pass), unconditionally sampling everything can produce very high volumes against Tempo and incur noticeable per-span overhead under load. Consider making the sampler configurable (e.g.OTEL_TRACES_SAMPLER/OTEL_TRACES_SAMPLER_ARG, whichsdktrace.NewTracerProviderdoes not read by default) or defaulting toParentBased(TraceIDRatioBased(<ratio>))with an env-driven ratio.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/common/telemetry/tracing.go` around lines 36 - 40, The TracerProvider currently uses sdktrace.ParentBased(sdktrace.AlwaysSample()) which will sample and export every span; change tracing.go so the sampler is configurable and defaults to a ratio-based parent sampler: read an env var (e.g. OTEL_TRACES_SAMPLER_ARG or a custom TRACE_SAMPLING_RATIO), parse it to a float (with a safe default like 0.1), then build the provider with sdktrace.ParentBased(sdktrace.TraceIDRatioBased(ratio)) instead of AlwaysSample; modify the sdktrace.NewTracerProvider call that creates tp to accept this sampler so you can control sampling under load.docs/observability-setup.md (1)
519-526: CSV patching will drift on operator upgrade.Patching the CSV directly is documented as a workaround, but each subsequent OLM upgrade replaces the CSV and drops the appended env var, silently disabling tracing. Worth calling this out explicitly in the doc, and ideally pointing readers at a more durable mechanism (e.g. exposing
OTEL_EXPORTER_OTLP_ENDPOINTvia the OperatorConfig CR / Subscriptionconfig.env, which OLM preserves across upgrades).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/observability-setup.md` around lines 519 - 526, The docs currently recommend patching the CSV to add OTEL_EXPORTER_OTLP_ENDPOINT, which will be lost on OLM/operator upgrades; update the observability-setup.md guidance to explicitly call this out and replace the CSV-patch workaround with a durable approach recommendation: instruct users to set OTEL_EXPORTER_OTLP_ENDPOINT via the OperatorConfig custom resource (or via the Subscription's config.env) and show the high-level steps to expose that env var so OLM preserves it across upgrades; reference the CSV patch example and the OTEL_EXPORTER_OTLP_ENDPOINT symbol so readers can map the change.internal/controller/imagebuild/controller.go (1)
455-462: Use the new label constant here for consistency.
buildLabelsnow sources its keys fromapi/v1alpha1(e.g.LabelImageBuildName), but thisclient.MatchingLabelslookup still hardcodes the literal string"automotive.sdv.cloud.redhat.com/imagebuild-name". If the constant is ever renamed, this lookup will silently miss every PipelineRun. Same applies to theapp.kubernetes.io/managed-by/automotive.sdv.cloud.redhat.com/imagebuild-namestrings used increateUploadPod(Line 2386–2388) andgetOrCreateWorkspacePVC(Line 2743–2744).♻️ Suggested change
- client.MatchingLabels{ - "automotive.sdv.cloud.redhat.com/imagebuild-name": imageBuild.Name, - }); err != nil { + client.MatchingLabels{ + automotivev1alpha1.LabelImageBuildName: imageBuild.Name, + }); err != nil {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/controller/imagebuild/controller.go` around lines 455 - 462, Replace hardcoded label keys with the shared label constants from api/v1alpha1: instead of the literal "automotive.sdv.cloud.redhat.com/imagebuild-name" use v1alpha1.LabelImageBuildName (ensure the package is imported), and replace any hardcoded managed-by keys (like "app.kubernetes.io/managed-by" or the literal managed label) with the corresponding constant(s) from api/v1alpha1; apply this change in the PipelineRun listing code snippet (the reconcile listing that builds pipelineRunList) and the other usages in createUploadPod and getOrCreateWorkspacePVC so all client.MatchingLabels lookups use the centralized constants (e.g., LabelImageBuildName / the managed-by constant) rather than string literals.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/v1alpha1/labels.go`:
- Around line 6-13: LabelTraceID and AnnotationTraceID currently share the same
string key which can silently break if trace ID formats later violate label
constraints; either (A) explicitly separate the keys (change one constant to a
distinct label key) and update any copy logic to use the new constant, or (B)
document the contract and add validation/normalization where the annotation is
copied to a label (validate/transform the UUID in the code path that reads
AnnotationTraceID and sets LabelTraceID) so label-safe characters and length are
guaranteed; reference the constants LabelTraceID and AnnotationTraceID and the
code path that copies annotation -> label when applying this change.
In `@cmd/main.go`:
- Around line 104-106: The change sets zap.Options (opts) Development to false
which enables production JSON encoding and sampling; update release notes and
dev docs to call this out and any tooling that greps human-readable logs, and
add a brief inline comment near the opts := zap.Options{Development: false}
assignment explaining the intentional switch to production logger and how to
override it (e.g., via a local/dev env flag) so operators/devs know how to
restore human-readable logs for local debugging.
In `@docs/observability-setup.md`:
- Around line 394-411: The cleanup step misses the serial file created by the
openssl -CAcreateserial flag; update the final cleanup command that currently
removes /tmp/tempo-ca.crt /tmp/tempo-ca.key /tmp/otel-client.* to also remove
the serial file (e.g., /tmp/tempo-ca.srl) so the doc’s copy/paste sequence
leaves no stray files after generating the client cert; locate the openssl x509
command and the subsequent rm line in the docs/observability-setup.md snippet
and add the serial filename to the rm list.
- Around line 16-34: The fenced diagram block in docs/observability-setup.md
lacks a language tag causing markdownlint MD040; update the opening fence from
``` to ```text so the ASCII diagram is treated as plain text (leave the closing
``` unchanged) to silence the lint warning.
In `@go.mod`:
- Line 53: The go.mod lists github.com/go-jose/go-jose/v4 at v4.1.3 which is
vulnerable; update that module to v4.1.4 or later to patch GHSA-78h2-9frx-2jm8
by either bumping the dependency that imports github.com/go-jose/go-jose/v4 or
adding a go.mod replace/require entry for github.com/go-jose/go-jose/v4 v4.1.4+
and then run go get ./... and go mod tidy to ensure the newer version is
resolved and vendor/mod files are updated; verify no other indirect constraints
pin it back to v4.1.3.
- Line 175: The go.mod currently pins google.golang.org/grpc to v1.79.2 which is
vulnerable (GO-2026-4762); update the grpc module version in go.mod to v1.79.3
or newer, run go mod tidy to refresh go.sum, and ensure any CI/builds pick up
the new dependency; look for the google.golang.org/grpc entry in go.mod and
replace v1.79.2 with v1.79.3 (or later) then commit the updated go.mod and
go.sum.
In `@internal/common/telemetry/tracing_test.go`:
- Around line 33-60: TestInitTracing_WithEndpoint mutates global OTel state via
otel.SetTracerProvider and otel.SetTextMapPropagator and does not restore them,
causing flakiness for other tests (including TestInitTracing_NoEndpoint); before
calling InitTracing in TestInitTracing_WithEndpoint (and defensively in
TestInitTracing_NoEndpoint) capture the current global state by reading
otel.GetTracerProvider() and otel.GetTextMapPropagator(), then register a
t.Cleanup that restores those values via otel.SetTracerProvider(oldTP) and
otel.SetTextMapPropagator(oldProp) so the global provider/propagator are
returned to their prior values after the test finishes while leaving the rest of
the test logic (InitTracing/shutdown checks and span assertions) unchanged.
In `@internal/common/telemetry/tracing.go`:
- Around line 17-46: InitTracing currently creates the OTLP gRPC exporter with
default TLS (otlptracegrpc.New) and returns tp.Shutdown without timeout; change
InitTracing (function InitTracing) to construct the exporter with
otlptracegrpc.WithInsecure() when the endpoint is plaintext (or respect
OTEL_EXPORTER_OTLP_INSECURE env var) so it won’t attempt TLS to a plaintext
collector, and update the shutdown call site (where shutdownTracing is invoked,
e.g., cmd/main.go) to wrap tp.Shutdown with a context with a bounded timeout
(use context.WithTimeout and call Shutdown with that context) so spans are
flushed within the process grace period; also update the test in tracing_test.go
to assert timeout-aware shutdown behavior.
In `@internal/controller/imagebuild/controller.go`:
- Around line 1077-1083: The Tekton param "trace-id" is being set with
getTraceID(imageBuild) even when it returns an empty string, causing empty
ADO_TRACE_ID/LabelTraceID; update the places that set the "trace-id" Param (the
blocks around the trace-id param and the similar spots at the other line ranges)
to first obtain a non-empty trace ID: call or reuse ensureTraceID(imageBuild) or
the same ID-generation logic to lazily create a trace ID if getTraceID returns
"" and persist it back to the ImageBuild annotations (or return a requeueable
error if you prefer strictness), then use that non-empty value for the Param so
no PipelineRun/TaskRun is created with an empty correlation ID. Ensure you
reference getTraceID and ensureTraceID/Reconcile to persist the new ID.
- Around line 197-200: The root span is being annotated with imagebuild.trace_id
before the trace ID is created, so on the first reconcile the attribute is
empty; update the reconcile flow so the span.SetAttributes call uses the trace
ID after it is ensured—either move the span.SetAttributes call to after
handleInitialState/ensureTraceID returns or call span.SetAttributes again with
attribute.String("imagebuild.trace_id", getTraceID(imageBuild)) immediately
after handleInitialState/ensureTraceID to guarantee the trace_id is populated
(refer to span.SetAttributes, getTraceID, and handleInitialState/ensureTraceID
in your changes).
- Around line 354-359: The current logic only calls ensureTraceID inside the
empty-phase branch, so ImageBuilds created before this change never get a
trace-id; move the ensureTraceID + r.Update logic to the start of Reconcile (or
add a short pre-check at the top of the Reconcile handler) so that if
getTraceID(imageBuild) == "" you call ensureTraceID(imageBuild) and persist it
with r.Update(ctx, imageBuild) before proceeding with phase-specific handlers;
keep the same error handling (fmt.Errorf("failed to set trace-id annotation:
%w", err)) and ensure you only run this when the annotation is absent to avoid
redundant updates.
In `@internal/controller/workspace/controller.go`:
- Around line 437-443: The current literal Labels map sets
automotivev1alpha1.LabelOwner to
controllerutils.SanitizeLabelValue(ws.Spec.Owner) which emits an empty string
label when Owner is unset; change to omit the owner entry from the literal map
and after the map is created compute owner :=
controllerutils.SanitizeLabelValue(ws.Spec.Owner) and only add pod/obj
labels[automotivev1alpha1.LabelOwner] = owner if owner != ""; keep using
controllerutils.SanitizeLabelValue and the same label key so locating the change
around the Labels map and where pod.Labels (or the labels variable) is populated
is straightforward.
---
Nitpick comments:
In `@cmd/main.go`:
- Around line 117-126: The deferred shutdownTracing call is bypassed by any
os.Exit path in main; refactor main by extracting the startup and runtime logic
into a run() error function (e.g., move mode validation and the rest of main's
logic into run), let main call run() and capture its error, and perform
shutdownTracing in main after run returns (or in a deferred in main that runs
because you no longer call os.Exit from inside run); ensure all early-return
errors from run return an error instead of calling os.Exit directly so
shutdownTracing(shutdownCtx) is always executed before exiting.
In `@docs/observability-setup.md`:
- Around line 519-526: The docs currently recommend patching the CSV to add
OTEL_EXPORTER_OTLP_ENDPOINT, which will be lost on OLM/operator upgrades; update
the observability-setup.md guidance to explicitly call this out and replace the
CSV-patch workaround with a durable approach recommendation: instruct users to
set OTEL_EXPORTER_OTLP_ENDPOINT via the OperatorConfig custom resource (or via
the Subscription's config.env) and show the high-level steps to expose that env
var so OLM preserves it across upgrades; reference the CSV patch example and the
OTEL_EXPORTER_OTLP_ENDPOINT symbol so readers can map the change.
In `@internal/common/tasks/tasks.go`:
- Around line 1505-1572: GeneratePrepareBuilderTask and the sealedTaskSpec are
missing the trace-id param/env so their containers don't get ADO_TRACE_ID for
log correlation; add traceIDParamSpec() to the TaskSpec Params and include
traceIDEnvVar() in the task container/env definitions (e.g., where Params are
declared in GeneratePrepareBuilderTask and in sealedTaskSpec container/env
setup) so the generated Tekton Task accepts a trace-id and sets ADO_TRACE_ID for
all task steps; if you intentionally exclude tracing, add a brief comment in
GeneratePrepareBuilderTask and sealedTaskSpec explaining why the trace-id is
omitted.
In `@internal/common/telemetry/tracing_test.go`:
- Around line 36-46: The test currently forces a live OTLP gRPC connection by
setting OTEL_EXPORTER_OTLP_ENDPOINT to http://localhost:4317 which causes
InitTracing (and its returned shutdown) to block on gRPC dial timeouts; change
the test to avoid a real dial by either (A) creating a local in-process test
collector (e.g., httptest.Listener/Server) and point OTEL_EXPORTER_OTLP_ENDPOINT
to that, or (B) add a test seam to InitTracing to accept an exporter factory or
a flag to install a no-op/test exporter, and call InitTracing with that in the
test; update the test to use the new seam (or local server) so shutdown returns
quickly and no background gRPC dial is attempted (referencing InitTracing,
shutdown and OTEL_EXPORTER_OTLP_ENDPOINT).
In `@internal/common/telemetry/tracing.go`:
- Around line 36-40: The TracerProvider currently uses
sdktrace.ParentBased(sdktrace.AlwaysSample()) which will sample and export every
span; change tracing.go so the sampler is configurable and defaults to a
ratio-based parent sampler: read an env var (e.g. OTEL_TRACES_SAMPLER_ARG or a
custom TRACE_SAMPLING_RATIO), parse it to a float (with a safe default like
0.1), then build the provider with
sdktrace.ParentBased(sdktrace.TraceIDRatioBased(ratio)) instead of AlwaysSample;
modify the sdktrace.NewTracerProvider call that creates tp to accept this
sampler so you can control sampling under load.
In `@internal/controller/imagebuild/controller.go`:
- Around line 455-462: Replace hardcoded label keys with the shared label
constants from api/v1alpha1: instead of the literal
"automotive.sdv.cloud.redhat.com/imagebuild-name" use
v1alpha1.LabelImageBuildName (ensure the package is imported), and replace any
hardcoded managed-by keys (like "app.kubernetes.io/managed-by" or the literal
managed label) with the corresponding constant(s) from api/v1alpha1; apply this
change in the PipelineRun listing code snippet (the reconcile listing that
builds pipelineRunList) and the other usages in createUploadPod and
getOrCreateWorkspacePVC so all client.MatchingLabels lookups use the centralized
constants (e.g., LabelImageBuildName / the managed-by constant) rather than
string literals.
In `@internal/controller/workspace/controller.go`:
- Around line 64-138: The spans created in Reconcile (wsTracer.Start) and in
ensurePVC and ensurePod do not record errors or set span status on failure;
change each function to use a named error return (e.g., retErr error) and
replace the simple defer span.End() with a defer that checks retErr and, if
non-nil, calls span.RecordError(retErr) and span.SetStatus(codes.Error,
retErr.Error()) before ending the span (import go.opentelemetry.io/otel/codes).
Apply this pattern to the functions named Reconcile, ensurePVC, and ensurePod so
tracing reflects actual error outcomes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dfe37d5d-fdb4-48d9-ac19-0d47de861cb0
⛔ Files ignored due to path filters (194)
go.sumis excluded by!**/*.sum,!**go.sumvendor/cel.dev/expr/BUILD.bazelis excluded by!vendor/**vendor/cel.dev/expr/MODULE.bazelis excluded by!vendor/**vendor/cel.dev/expr/checked.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/cel.dev/expr/eval.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/cel.dev/expr/explain.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/cel.dev/expr/syntax.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/cel.dev/expr/value.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/github.com/go-jose/go-jose/v4/README.mdis excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/crypter.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/shared.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/signing.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/symmetric.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/symmetric_legacy.gois excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/BUILD.bazelis excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/context.gois excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/handler.gois excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/marshal_jsonpb.gois excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/mux.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/LICENSEis excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/exporter.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform/attribute.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform/instrumentation.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform/resource.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform/span.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/LICENSEis excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/client.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/counter/counter.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/gen.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/observ/doc.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/observ/instrumentation.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/observ/target.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/partialsuccess.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/retry/retry.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/version.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/x/README.mdis excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/x/observ.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/x/x.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/version.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/LICENSEis excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/internal/x/features.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/internal/x/x.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/builtin.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/container.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/env.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_bsd.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_linux.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_readfile.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_unsupported.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_windows.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os_release_unix.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os_unix.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os_unsupported.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/process.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/resource.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/batch_span_processor.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/doc.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/id_generator.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/env/env.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/batch_span_processor.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/doc.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/simple_span_processor.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/tracer.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/provider.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/sampling.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/simple_span_processor.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/snapshot.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/span.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/span_limits.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/tracer.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/version.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/version.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.39.0/otelconv/metric.gois excluded by!vendor/**vendor/go.opentelemetry.io/proto/otlp/common/v1/common.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/go.opentelemetry.io/proto/otlp/resource/v1/resource.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/go.opentelemetry.io/proto/otlp/trace/v1/trace.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/golang.org/x/crypto/scrypt/scrypt.gois excluded by!vendor/**vendor/golang.org/x/net/html/node.gois excluded by!vendor/**vendor/golang.org/x/net/html/nodetype_string.gois excluded by!vendor/**vendor/golang.org/x/net/http2/client_priority_go126.gois excluded by!vendor/**vendor/golang.org/x/net/http2/client_priority_go127.gois excluded by!vendor/**vendor/golang.org/x/net/http2/frame.gois excluded by!vendor/**vendor/golang.org/x/net/http2/http2.gois excluded by!vendor/**vendor/golang.org/x/net/http2/server.gois excluded by!vendor/**vendor/golang.org/x/net/http2/transport.gois excluded by!vendor/**vendor/golang.org/x/net/http2/writesched_priority_rfc7540.gois excluded by!vendor/**vendor/golang.org/x/net/internal/httpsfv/httpsfv.gois excluded by!vendor/**vendor/golang.org/x/oauth2/deviceauth.gois excluded by!vendor/**vendor/golang.org/x/oauth2/oauth2.gois excluded by!vendor/**vendor/golang.org/x/oauth2/pkce.gois excluded by!vendor/**vendor/golang.org/x/oauth2/token.gois excluded by!vendor/**vendor/golang.org/x/oauth2/transport.gois excluded by!vendor/**vendor/golang.org/x/sys/cpu/cpu_arm64.gois excluded by!vendor/**vendor/golang.org/x/sys/cpu/cpu_other_arm64.gois excluded by!vendor/**vendor/golang.org/x/sys/cpu/cpu_windows_arm64.gois excluded by!vendor/**vendor/golang.org/x/sys/unix/ioctl_signed.gois excluded by!vendor/**vendor/golang.org/x/sys/unix/ioctl_unsigned.gois excluded by!vendor/**vendor/golang.org/x/sys/unix/syscall_solaris.gois excluded by!vendor/**vendor/golang.org/x/sys/unix/syscall_unix.gois excluded by!vendor/**vendor/golang.org/x/sys/windows/syscall_windows.gois excluded by!vendor/**vendor/golang.org/x/sys/windows/types_windows.gois excluded by!vendor/**vendor/golang.org/x/sys/windows/zsyscall_windows.gois excluded by!vendor/**vendor/golang.org/x/text/message/catalog/catalog.gois excluded by!vendor/**vendor/golang.org/x/text/message/catalog/dict.gois excluded by!vendor/**vendor/golang.org/x/text/message/catalog/go19.gois excluded by!vendor/**vendor/golang.org/x/text/message/catalog/gopre19.gois excluded by!vendor/**vendor/golang.org/x/text/secure/bidirule/bidirule.gois excluded by!vendor/**vendor/golang.org/x/text/secure/bidirule/bidirule10.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/secure/bidirule/bidirule9.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables10.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables11.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables12.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables13.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables15.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables17.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables9.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/forminfo.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables10.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables11.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables12.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables15.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables17.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables9.0.0.gois excluded by!vendor/**vendor/google.golang.org/grpc/CONTRIBUTING.mdis excluded by!vendor/**vendor/google.golang.org/grpc/balancer/balancer.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/pickfirst/internal/internal.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/pickfirst/pickfirst.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/pickfirst/pickfirstleaf/pickfirstleaf.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/roundrobin/roundrobin.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/subconn.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer_wrapper.gois excluded by!vendor/**vendor/google.golang.org/grpc/binarylog/grpc_binarylog_v1/binarylog.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/google.golang.org/grpc/clientconn.gois excluded by!vendor/**vendor/google.golang.org/grpc/credentials/credentials.gois excluded by!vendor/**vendor/google.golang.org/grpc/credentials/tls.gois excluded by!vendor/**vendor/google.golang.org/grpc/encoding/encoding.gois excluded by!vendor/**vendor/google.golang.org/grpc/encoding/gzip/gzip.gois excluded by!vendor/**vendor/google.golang.org/grpc/encoding/internal/internal.gois excluded by!vendor/**vendor/google.golang.org/grpc/encoding/proto/proto.gois excluded by!vendor/**vendor/google.golang.org/grpc/experimental/stats/metricregistry.gois excluded by!vendor/**vendor/google.golang.org/grpc/experimental/stats/metrics.gois excluded by!vendor/**vendor/google.golang.org/grpc/health/grpc_health_v1/health.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/google.golang.org/grpc/health/grpc_health_v1/health_grpc.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/google.golang.org/grpc/interceptor.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/balancer/gracefulswitch/gracefulswitch.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/balancer/weight/weight.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/buffer/unbounded.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/channelz/trace.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/envconfig/envconfig.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/envconfig/xds.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/experimental.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/grpcsync/callback_serializer.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/idle/idle.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/internal.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/resolver/delegatingresolver/delegatingresolver.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/resolver/dns/dns_resolver.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/stats/metrics_recorder_list.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/stats/stats.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/client_stream.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/controlbuf.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/flowcontrol.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/handler_server.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/http2_client.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/http2_server.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/http_util.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/server_stream.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/transport.gois excluded by!vendor/**vendor/google.golang.org/grpc/mem/buffer_pool.gois excluded by!vendor/**vendor/google.golang.org/grpc/mem/buffer_slice.gois excluded by!vendor/**vendor/google.golang.org/grpc/mem/buffers.gois excluded by!vendor/**vendor/google.golang.org/grpc/preloader.gois excluded by!vendor/**vendor/google.golang.org/grpc/resolver/resolver.gois excluded by!vendor/**vendor/google.golang.org/grpc/resolver_wrapper.gois excluded by!vendor/**vendor/google.golang.org/grpc/rpc_util.gois excluded by!vendor/**vendor/google.golang.org/grpc/server.gois excluded by!vendor/**vendor/google.golang.org/grpc/stream.gois excluded by!vendor/**vendor/google.golang.org/grpc/version.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/encoding/tag/tag.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/encoding/text/decode.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/filedesc/desc.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/filedesc/desc_lazy.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/genid/descriptor_gen.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/impl/codec_map.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/impl/decode.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/impl/validate.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/version/version.gois excluded by!vendor/**vendor/google.golang.org/protobuf/proto/decode.gois excluded by!vendor/**vendor/google.golang.org/protobuf/reflect/protodesc/desc.gois excluded by!vendor/**vendor/google.golang.org/protobuf/reflect/protodesc/editions.gois excluded by!vendor/**vendor/google.golang.org/protobuf/types/descriptorpb/descriptor.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/google.golang.org/protobuf/types/known/timestamppb/timestamp.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/modules.txtis excluded by!vendor/**
📒 Files selected for processing (11)
api/v1alpha1/labels.gocmd/main.godocs/observability-setup.mdgo.modinternal/common/tasks/tasks.gointernal/common/telemetry/tracing.gointernal/common/telemetry/tracing_test.gointernal/controller/controllerutils/labels.gointernal/controller/controllerutils/labels_test.gointernal/controller/imagebuild/controller.gointernal/controller/workspace/controller.go
bennyz
force-pushed
the
worktree-monitoring-config
branch
from
April 26, 2026 19:13
0cd0d4c to
65e0822
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
go.mod (2)
175-175:⚠️ Potential issue | 🔴 Critical
google.golang.org/grpc v1.79.2still pinned — bump to ≥ v1.79.3 (GHSA-p77j-4mvh-x3m3).Critical authorization bypass via missing leading slash in
:path. Patched in v1.79.3.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` at line 175, The go.mod currently pins google.golang.org/grpc at v1.79.2 which is vulnerable; update the module requirement for google.golang.org/grpc (the line containing "google.golang.org/grpc v1.79.2") to v1.79.3 or later and run your Go tooling (e.g., go get google.golang.org/grpc@v1.79.3 && go mod tidy) to propagate the change and regenerate go.sum so the fixed version is used.
53-53:⚠️ Potential issue | 🟠 MajorVulnerable
github.com/go-jose/go-jose/v4 v4.1.3— bump to ≥ v4.1.4 (GHSA-78h2-9frx-2jm8).OSV still flags v4.1.3 (Go JOSE panic on JWE decryption). Patched in v4.1.4. Update via
go geton the importing dependency or add areplacedirective.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` at line 53, The go.mod currently pins the vulnerable module github.com/go-jose/go-jose/v4 at v4.1.3; update this to v4.1.4 or later to include the JWE decryption panic fix by running `go get github.com/go-jose/go-jose/v4@v4.1.4` (or updating the direct importing dependency), or add a replace directive to ensure github.com/go-jose/go-jose/v4 resolves to >= v4.1.4 so the indirect entry is upgraded in the module graph.
🧹 Nitpick comments (3)
internal/common/tasks/tasks.go (1)
610-650: Optional:find-manifest-filestep doesn't getADO_TRACE_ID.Only the
build-imagestep (line 649) getstraceIDEnvVar()injected. The precedingfind-manifest-filestep (lines 611-621) runs withoutADO_TRACE_ID, so any logs it emits won't be correlatable via the env var (they'll still carry the pod label, so this is mostly cosmetic). Consider addingEnv: []corev1.EnvVar{traceIDEnvVar()}to that step for consistency if you want every step's stdout to carry the trace-id when, e.g., scripts choose to emit it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/common/tasks/tasks.go` around lines 610 - 650, The find-manifest-file Tekton step currently lacks the trace env var so its logs can't include ADO_TRACE_ID; update the step definition (the Step with Name "find-manifest-file") to include an Env field that injects traceIDEnvVar() (same helper used in the "build-image" step) — e.g., add Env: []corev1.EnvVar{traceIDEnvVar()} to that step so all steps consistently get the ADO_TRACE_ID.api/v1alpha1/operatorconfig_types.go (2)
491-497: SimplifyGetEndpoint.The
c.Endpoint != ""guard is redundant —c.Endpoint's zero value is already"". The function reduces to a nil-receiver check.♻️ Suggested simplification
// GetEndpoint returns the configured endpoint or empty string func (c *TracingConfig) GetEndpoint() string { - if c != nil && c.Endpoint != "" { + if c != nil { return c.Endpoint } return "" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/v1alpha1/operatorconfig_types.go` around lines 491 - 497, The GetEndpoint method on TracingConfig has a redundant non-empty string check; simplify it to only guard against a nil receiver: in the GetEndpoint function (type TracingConfig) return c.Endpoint when c is non-nil and return "" otherwise, removing the c.Endpoint != "" condition so the method becomes a simple nil check and direct return of c.Endpoint.
482-489:GetSamplingRatiosilently swallows parse errors and returns 1.0 (full sampling).The kubebuilder validation pattern
^(0(\.\d+)?|1(\.0+)?)$should prevent invalid values at admission, but if a malformed value ever slips through (e.g., older CRDs without the pattern, or akubectl --validate=falseapply), defaulting to 1.0 silently turns a misconfiguration into "trace everything" rather than a no-op or a logged warning. Consider logging a warning on parse failure so operators notice the misconfig.Also:
0is permitted by the regex and parses to 0.0 (no sampling), which is the correct behavior — just calling this out as an intentional difference from the "unset → 1.0" default.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/v1alpha1/operatorconfig_types.go` around lines 482 - 489, The GetSamplingRatio function currently swallows strconv.ParseFloat errors and returns 1.0 silently; modify GetSamplingRatio to log a warning when ParseFloat fails (so operators are alerted to a malformed SamplingRatio) while preserving existing behavior for valid values (including "0" → 0.0) — for example, when strconv.ParseFloat returns an error in TracingConfig.GetSamplingRatio, emit a warning via the repo's logging facility (e.g., klog.Warningf or the package logger) including the invalid c.SamplingRatio and the parse error, then return the fallback 1.0.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/observability-setup.md`:
- Around line 516-527: The current JSON-patch using oc patch with op: "add" to
path "/spec/install/spec/deployments/0/spec/template/spec/containers/0/env/-"
unconditionally appends OTEL_EXPORTER_OTLP_ENDPOINT and is non-idempotent;
change the recipe to first select the single Succeeded CSV (instead of using `oc
get csv ... | grep automotive-dev-operator`) and perform a safe
read-modify-write or an atomic JSON patch that tests/replaces an indexed env
entry: fetch the CSV (CSV variable), inspect the container env array for an
existing name "OTEL_EXPORTER_OTLP_ENDPOINT", and if present replace its value
(use a JSON-patch "test" + "replace" on the specific index) otherwise add it at
a known index; ensure your patch targets the specific container via its name or
index to avoid matching multiple CSVs during upgrades.
- Around line 388-392: Replace the incorrect secret-based extraction commands
(the lines referencing "oc get secret tempo-tempo-signing-ca" and the keys
"ca.crt" / "ca.key") with a ConfigMap-based extraction: target the ConfigMap
named "<tempostack-name>-ca-bundle" (e.g., "tempo-tempo-ca-bundle") and read the
"service-ca.crt" data key, writing it to /tmp/tempo-ca.crt; remove any attempts
to read "ca.key" since the operator exposes only the CA bundle via the
service-ca.crt key so subsequent steps use the valid CA file.
In `@go.mod`:
- Line 159: Update the OpenTelemetry module versions in go.mod from v1.41.0 to
v1.43.0 to address GHSA-hfvc-g4fc-pqhx: specifically bump
go.opentelemetry.io/otel, go.opentelemetry.io/otel/sdk,
go.opentelemetry.io/otel/trace, go.opentelemetry.io/otel/metric and any otel
exporter modules like go.opentelemetry.io/otel/exporters/otlp/otlptrace* to
v1.43.0, then run the appropriate Go module commands (e.g., go get ./... or go
get go.opentelemetry.io/otel@v1.43.0 and go mod tidy) to update go.sum and
ensure all OTel modules are aligned at v1.43.0.
In `@internal/common/tasks/tasks.go`:
- Line 1056: Add unit tests that cover the new trace-id helpers: create tests
for traceIDParamSpec(), traceIDEnvVar(), and traceIDPipelineParam() to assert
their expected behavior and wiring; specifically, assert traceIDParamSpec()
returns the correct ParameterSpec (name, type, description/defaults),
traceIDEnvVar() produces the correct env var injection (env var name
ADO_TRACE_ID and valueFrom reference to the pipeline param), and
traceIDPipelineParam() produces the pipeline parameter binding used by
pipeline-task bindings. Place these tests in the existing *_test.go suite for
tasks (following the project's test naming conventions), mock or construct
minimal Pipeline/Task objects to verify that pipeline-task bindings include the
trace-id parameter and that the ADO_TRACE_ID env var is present and wired to the
pipeline param; run tests to ensure they fail first if behavior is
unimplemented, then implement changes until tests pass.
In `@internal/controller/operatorconfig/controller.go`:
- Around line 312-316: The status field is being set directly from the spec so
it can report tracing as enabled even though the operator process only
initializes tracing at startup; modify the update in controller.go (the
tracingEnabled logic that writes config.Status.TracingEnabled) to consult the
actual runtime tracing state exported from the telemetry package (e.g.,
telemetry.IsTracingInitialized() or telemetry.TracingInitialized) and only set
Status.TracingEnabled = true when both spec.Tracing.Enabled is true AND
telemetry reports tracing is initialized; alternatively, if you prefer the
"requires restart" approach, set a Status.Message/Condition on the
OperatorConfig (instead of flipping TracingEnabled) that explicitly states
tracing changes require a controller restart and leave TracingEnabled reflecting
the runtime telemetry flag.
---
Duplicate comments:
In `@go.mod`:
- Line 175: The go.mod currently pins google.golang.org/grpc at v1.79.2 which is
vulnerable; update the module requirement for google.golang.org/grpc (the line
containing "google.golang.org/grpc v1.79.2") to v1.79.3 or later and run your Go
tooling (e.g., go get google.golang.org/grpc@v1.79.3 && go mod tidy) to
propagate the change and regenerate go.sum so the fixed version is used.
- Line 53: The go.mod currently pins the vulnerable module
github.com/go-jose/go-jose/v4 at v4.1.3; update this to v4.1.4 or later to
include the JWE decryption panic fix by running `go get
github.com/go-jose/go-jose/v4@v4.1.4` (or updating the direct importing
dependency), or add a replace directive to ensure github.com/go-jose/go-jose/v4
resolves to >= v4.1.4 so the indirect entry is upgraded in the module graph.
---
Nitpick comments:
In `@api/v1alpha1/operatorconfig_types.go`:
- Around line 491-497: The GetEndpoint method on TracingConfig has a redundant
non-empty string check; simplify it to only guard against a nil receiver: in the
GetEndpoint function (type TracingConfig) return c.Endpoint when c is non-nil
and return "" otherwise, removing the c.Endpoint != "" condition so the method
becomes a simple nil check and direct return of c.Endpoint.
- Around line 482-489: The GetSamplingRatio function currently swallows
strconv.ParseFloat errors and returns 1.0 silently; modify GetSamplingRatio to
log a warning when ParseFloat fails (so operators are alerted to a malformed
SamplingRatio) while preserving existing behavior for valid values (including
"0" → 0.0) — for example, when strconv.ParseFloat returns an error in
TracingConfig.GetSamplingRatio, emit a warning via the repo's logging facility
(e.g., klog.Warningf or the package logger) including the invalid
c.SamplingRatio and the parse error, then return the fallback 1.0.
In `@internal/common/tasks/tasks.go`:
- Around line 610-650: The find-manifest-file Tekton step currently lacks the
trace env var so its logs can't include ADO_TRACE_ID; update the step definition
(the Step with Name "find-manifest-file") to include an Env field that injects
traceIDEnvVar() (same helper used in the "build-image" step) — e.g., add Env:
[]corev1.EnvVar{traceIDEnvVar()} to that step so all steps consistently get the
ADO_TRACE_ID.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 261a3622-beb7-4aad-b059-6b9b75bc2711
⛔ Files ignored due to path filters (195)
config/crd/bases/automotive.sdv.cloud.redhat.com_operatorconfigs.yamlis excluded by!config/crd/bases/**go.sumis excluded by!**/*.sum,!**go.sumvendor/cel.dev/expr/BUILD.bazelis excluded by!vendor/**vendor/cel.dev/expr/MODULE.bazelis excluded by!vendor/**vendor/cel.dev/expr/checked.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/cel.dev/expr/eval.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/cel.dev/expr/explain.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/cel.dev/expr/syntax.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/cel.dev/expr/value.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/github.com/go-jose/go-jose/v4/README.mdis excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/crypter.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/shared.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/signing.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/symmetric.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/symmetric_legacy.gois excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/BUILD.bazelis excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/context.gois excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/handler.gois excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/marshal_jsonpb.gois excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/mux.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/LICENSEis excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/exporter.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform/attribute.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform/instrumentation.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform/resource.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform/span.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/LICENSEis excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/client.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/counter/counter.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/gen.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/observ/doc.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/observ/instrumentation.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/observ/target.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/partialsuccess.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/retry/retry.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/version.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/x/README.mdis excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/x/observ.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/x/x.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/version.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/LICENSEis excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/internal/x/features.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/internal/x/x.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/builtin.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/container.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/env.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_bsd.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_linux.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_readfile.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_unsupported.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_windows.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os_release_unix.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os_unix.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os_unsupported.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/process.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/resource.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/batch_span_processor.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/doc.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/id_generator.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/env/env.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/batch_span_processor.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/doc.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/simple_span_processor.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/tracer.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/provider.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/sampling.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/simple_span_processor.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/snapshot.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/span.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/span_limits.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/tracer.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/version.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/version.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.39.0/otelconv/metric.gois excluded by!vendor/**vendor/go.opentelemetry.io/proto/otlp/common/v1/common.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/go.opentelemetry.io/proto/otlp/resource/v1/resource.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/go.opentelemetry.io/proto/otlp/trace/v1/trace.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/golang.org/x/crypto/scrypt/scrypt.gois excluded by!vendor/**vendor/golang.org/x/net/html/node.gois excluded by!vendor/**vendor/golang.org/x/net/html/nodetype_string.gois excluded by!vendor/**vendor/golang.org/x/net/http2/client_priority_go126.gois excluded by!vendor/**vendor/golang.org/x/net/http2/client_priority_go127.gois excluded by!vendor/**vendor/golang.org/x/net/http2/frame.gois excluded by!vendor/**vendor/golang.org/x/net/http2/http2.gois excluded by!vendor/**vendor/golang.org/x/net/http2/server.gois excluded by!vendor/**vendor/golang.org/x/net/http2/transport.gois excluded by!vendor/**vendor/golang.org/x/net/http2/writesched_priority_rfc7540.gois excluded by!vendor/**vendor/golang.org/x/net/internal/httpsfv/httpsfv.gois excluded by!vendor/**vendor/golang.org/x/oauth2/deviceauth.gois excluded by!vendor/**vendor/golang.org/x/oauth2/oauth2.gois excluded by!vendor/**vendor/golang.org/x/oauth2/pkce.gois excluded by!vendor/**vendor/golang.org/x/oauth2/token.gois excluded by!vendor/**vendor/golang.org/x/oauth2/transport.gois excluded by!vendor/**vendor/golang.org/x/sys/cpu/cpu_arm64.gois excluded by!vendor/**vendor/golang.org/x/sys/cpu/cpu_other_arm64.gois excluded by!vendor/**vendor/golang.org/x/sys/cpu/cpu_windows_arm64.gois excluded by!vendor/**vendor/golang.org/x/sys/unix/ioctl_signed.gois excluded by!vendor/**vendor/golang.org/x/sys/unix/ioctl_unsigned.gois excluded by!vendor/**vendor/golang.org/x/sys/unix/syscall_solaris.gois excluded by!vendor/**vendor/golang.org/x/sys/unix/syscall_unix.gois excluded by!vendor/**vendor/golang.org/x/sys/windows/syscall_windows.gois excluded by!vendor/**vendor/golang.org/x/sys/windows/types_windows.gois excluded by!vendor/**vendor/golang.org/x/sys/windows/zsyscall_windows.gois excluded by!vendor/**vendor/golang.org/x/text/message/catalog/catalog.gois excluded by!vendor/**vendor/golang.org/x/text/message/catalog/dict.gois excluded by!vendor/**vendor/golang.org/x/text/message/catalog/go19.gois excluded by!vendor/**vendor/golang.org/x/text/message/catalog/gopre19.gois excluded by!vendor/**vendor/golang.org/x/text/secure/bidirule/bidirule.gois excluded by!vendor/**vendor/golang.org/x/text/secure/bidirule/bidirule10.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/secure/bidirule/bidirule9.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables10.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables11.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables12.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables13.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables15.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables17.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables9.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/forminfo.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables10.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables11.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables12.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables15.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables17.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables9.0.0.gois excluded by!vendor/**vendor/google.golang.org/grpc/CONTRIBUTING.mdis excluded by!vendor/**vendor/google.golang.org/grpc/balancer/balancer.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/pickfirst/internal/internal.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/pickfirst/pickfirst.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/pickfirst/pickfirstleaf/pickfirstleaf.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/roundrobin/roundrobin.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/subconn.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer_wrapper.gois excluded by!vendor/**vendor/google.golang.org/grpc/binarylog/grpc_binarylog_v1/binarylog.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/google.golang.org/grpc/clientconn.gois excluded by!vendor/**vendor/google.golang.org/grpc/credentials/credentials.gois excluded by!vendor/**vendor/google.golang.org/grpc/credentials/tls.gois excluded by!vendor/**vendor/google.golang.org/grpc/encoding/encoding.gois excluded by!vendor/**vendor/google.golang.org/grpc/encoding/gzip/gzip.gois excluded by!vendor/**vendor/google.golang.org/grpc/encoding/internal/internal.gois excluded by!vendor/**vendor/google.golang.org/grpc/encoding/proto/proto.gois excluded by!vendor/**vendor/google.golang.org/grpc/experimental/stats/metricregistry.gois excluded by!vendor/**vendor/google.golang.org/grpc/experimental/stats/metrics.gois excluded by!vendor/**vendor/google.golang.org/grpc/health/grpc_health_v1/health.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/google.golang.org/grpc/health/grpc_health_v1/health_grpc.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/google.golang.org/grpc/interceptor.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/balancer/gracefulswitch/gracefulswitch.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/balancer/weight/weight.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/buffer/unbounded.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/channelz/trace.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/envconfig/envconfig.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/envconfig/xds.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/experimental.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/grpcsync/callback_serializer.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/idle/idle.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/internal.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/resolver/delegatingresolver/delegatingresolver.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/resolver/dns/dns_resolver.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/stats/metrics_recorder_list.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/stats/stats.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/client_stream.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/controlbuf.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/flowcontrol.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/handler_server.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/http2_client.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/http2_server.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/http_util.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/server_stream.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/transport.gois excluded by!vendor/**vendor/google.golang.org/grpc/mem/buffer_pool.gois excluded by!vendor/**vendor/google.golang.org/grpc/mem/buffer_slice.gois excluded by!vendor/**vendor/google.golang.org/grpc/mem/buffers.gois excluded by!vendor/**vendor/google.golang.org/grpc/preloader.gois excluded by!vendor/**vendor/google.golang.org/grpc/resolver/resolver.gois excluded by!vendor/**vendor/google.golang.org/grpc/resolver_wrapper.gois excluded by!vendor/**vendor/google.golang.org/grpc/rpc_util.gois excluded by!vendor/**vendor/google.golang.org/grpc/server.gois excluded by!vendor/**vendor/google.golang.org/grpc/stream.gois excluded by!vendor/**vendor/google.golang.org/grpc/version.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/encoding/tag/tag.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/encoding/text/decode.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/filedesc/desc.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/filedesc/desc_lazy.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/genid/descriptor_gen.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/impl/codec_map.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/impl/decode.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/impl/validate.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/version/version.gois excluded by!vendor/**vendor/google.golang.org/protobuf/proto/decode.gois excluded by!vendor/**vendor/google.golang.org/protobuf/reflect/protodesc/desc.gois excluded by!vendor/**vendor/google.golang.org/protobuf/reflect/protodesc/editions.gois excluded by!vendor/**vendor/google.golang.org/protobuf/types/descriptorpb/descriptor.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/google.golang.org/protobuf/types/known/timestamppb/timestamp.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/modules.txtis excluded by!vendor/**
📒 Files selected for processing (14)
api/v1alpha1/labels.goapi/v1alpha1/operatorconfig_types.goapi/v1alpha1/zz_generated.deepcopy.gocmd/main.godocs/observability-setup.mdgo.modinternal/common/tasks/tasks.gointernal/common/telemetry/tracing.gointernal/common/telemetry/tracing_test.gointernal/controller/controllerutils/labels.gointernal/controller/controllerutils/labels_test.gointernal/controller/imagebuild/controller.gointernal/controller/operatorconfig/controller.gointernal/controller/workspace/controller.go
✅ Files skipped from review due to trivial changes (2)
- api/v1alpha1/labels.go
- internal/controller/imagebuild/controller.go
🚧 Files skipped from review as they are similar to previous changes (6)
- internal/controller/controllerutils/labels.go
- internal/controller/controllerutils/labels_test.go
- internal/common/telemetry/tracing.go
- internal/common/telemetry/tracing_test.go
- internal/controller/workspace/controller.go
- cmd/main.go
| go.opentelemetry.io/otel/sdk v1.37.0 // indirect | ||
| go.opentelemetry.io/otel/trace v1.41.0 // indirect | ||
| go.opentelemetry.io/proto/otlp v1.7.0 // indirect | ||
| go.opentelemetry.io/otel/sdk v1.41.0 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the latest stable version of go.opentelemetry.io/otel and otel/sdk modules?
💡 Result:
The latest stable version of both go.opentelemetry.io/otel and go.opentelemetry.io/otel/sdk modules is v1.43.0, released on 2026-04-03.
Citations:
- 1: https://pkg.go.dev/go.opentelemetry.io/otel
- 2: https://pkg.go.dev/go.opentelemetry.io/otel/sdk
- 3: https://github.com/open-telemetry/opentelemetry-go/releases/tag/v1.43.0
- 4: https://github.com/open-telemetry/opentelemetry-go/blob/main/versions.yaml
🏁 Script executed:
cat -n go.mod | grep -i "opentelemetry"Repository: centos-automotive-suite/automotive-dev-operator
Length of output: 647
🌐 Web query:
GHSA-hfvc-g4fc-pqhx OpenTelemetry vulnerability advisory
💡 Result:
GHSA-hfvc-g4fc-pqhx is a high-severity vulnerability (CVSS 7.3) in the OpenTelemetry Go SDK (go.opentelemetry.io/otel/sdk). It is an incomplete fix for the prior PATH hijacking vulnerability CVE-2026-24051 (GHSA-9h8m-3fm2-qjrq). While the fix updated the Darwin 'ioreg' command to use an absolute path, it left the BSD 'kenv' command using a bare name ("kenv"), enabling PATH hijacking on BSD (FreeBSD, NetBSD, OpenBSD, DragonFly BSD) and Solaris platforms. Affected versions: >= v1.15.0 and <= v1.42.0. Patched in: v1.43.0 (released around April 2026), by changing to "/bin/kenv". Root cause: In sdk/resource/host_id.go, execCommand("kenv", "-q", "smbios.system.uuid") uses exec.Command which searches PATH. A malicious 'kenv' earlier in PATH executes arbitrary code during host resource detection (when /etc/hostid missing). Attack requires local access to modify PATH or place malicious binary. Impact: Arbitrary code execution with application privileges. CVE ID: CVE-2026-39883. CWE-426: Untrusted Search Path. Official advisory: GHSA-hfvc-g4fc-pqhx
Citations:
- 1: GHSA-hfvc-g4fc-pqhx
- 2: https://mondoo.com/vulnerability-intelligence/vulnerability/GHSA-hfvc-g4fc-pqhx
- 3: https://app.opencve.io/cve/CVE-2026-39883
- 4: GHSA-hfvc-g4fc-pqhx
Bump go.opentelemetry.io/otel/sdk to v1.43.0 to fix GHSA-hfvc-g4fc-pqhx (PATH hijacking via bare kenv on BSD/Solaris).
The vulnerability affects versions v1.15.0 through v1.42.0. Practical risk is low in Linux containers (BSD-specific), but patching clears the OSV finding and keeps the project current. Requires bumping all OTel modules together: otel, otel/sdk, otel/trace, otel/metric, and otel/exporters/otlp/otlptrace* are all currently at v1.41.0 and should move to v1.43.0.
🧰 Tools
🪛 OSV Scanner (2.3.5)
[HIGH] 159-159: go.opentelemetry.io/otel/sdk 1.41.0: opentelemetry-go: BSD kenv command not using absolute path enables PATH hijacking
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@go.mod` at line 159, Update the OpenTelemetry module versions in go.mod from
v1.41.0 to v1.43.0 to address GHSA-hfvc-g4fc-pqhx: specifically bump
go.opentelemetry.io/otel, go.opentelemetry.io/otel/sdk,
go.opentelemetry.io/otel/trace, go.opentelemetry.io/otel/metric and any otel
exporter modules like go.opentelemetry.io/otel/exporters/otlp/otlptrace* to
v1.43.0, then run the appropriate Go module commands (e.g., go get ./... or go
get go.opentelemetry.io/otel@v1.43.0 and go mod tidy) to update go.sum and
ensure all OTel modules are aligned at v1.43.0.
bennyz
force-pushed
the
worktree-monitoring-config
branch
2 times, most recently
from
April 27, 2026 05:18
034f274 to
2ba35df
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/buildapi/server.go (1)
2048-2093:⚠️ Potential issue | 🟠 Major
GET /v1/builds/:namedrops the persisted trace ID.The POST path writes
automotivev1alpha1.AnnotationTraceIDonto theImageBuild, but the GET response never maps that annotation back intoBuildResponse.TraceID. Clients can only see the trace ID at creation time, not when fetching the build later.Suggested fix
writeJSON(c, http.StatusOK, BuildResponse{ Name: build.Name, Phase: build.Status.Phase, Message: build.Status.Message, RequestedBy: build.Annotations["automotive.sdv.cloud.redhat.com/requested-by"], + TraceID: build.Annotations[automotivev1alpha1.AnnotationTraceID], StartTime: func() string { if build.Status.StartTime != nil { return build.Status.StartTime.Format(time.RFC3339) } return ""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/buildapi/server.go` around lines 2048 - 2093, The GET /v1/builds/:name response construction (the BuildResponse literal returned in the handler) currently omits the persisted trace ID; update the BuildResponse initialization to set TraceID from the ImageBuild annotation automotivev1alpha1.AnnotationTraceID (e.g., read build.Annotations[automotivev1alpha1.AnnotationTraceID] and assign it to the TraceID field) so the trace ID written at POST time is returned on subsequent GETs; modify the BuildResponse creation in the handler that calls writeJSON to include TraceID populated from build.Annotations.
♻️ Duplicate comments (5)
docs/observability-setup.md (3)
410-411:⚠️ Potential issue | 🟡 MinorCleanup misses the serial file created by
openssl -CAcreateserial.The
openssl x509 -req -CAcreateserialcommand at line 400 creates/tmp/tempo-ca.srl(the CA serial number file). The cleanup command only removes the cert and key files, leaving the.srlfile behind.📝 Suggested change
-rm /tmp/tempo-ca.crt /tmp/tempo-ca.key /tmp/otel-client.* +rm /tmp/tempo-ca.crt /tmp/tempo-ca.key /tmp/tempo-ca.srl /tmp/otel-client.*🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/observability-setup.md` around lines 410 - 411, The cleanup step currently removes "/tmp/tempo-ca.crt", "/tmp/tempo-ca.key" and "/tmp/otel-client.*" but misses the CA serial file created by "openssl -CAcreateserial" ("/tmp/tempo-ca.srl"); update the cleanup command that contains the "rm /tmp/tempo-ca.crt /tmp/tempo-ca.key /tmp/otel-client.*" invocation to also remove "/tmp/tempo-ca.srl" (or a pattern that includes the .srl file) so no serial file is left behind.
16-34:⚠️ Potential issue | 🟡 MinorAdd language identifier to the fenced code block.
The ASCII diagram block lacks a language tag, causing
markdownlintMD040 warnings. Addtextafter the opening fence so linters treat it as plain text.📝 Suggested change
-``` +```text ┌─────────────────────────────────────────┐ │ OCP Console (UIPlugins) │🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/observability-setup.md` around lines 16 - 34, The fenced ASCII diagram block is missing a language identifier causing markdownlint MD040; update the opening code fence for the ASCII diagram (the triple backtick that begins the ASCII art block) to include the language label "text" (i.e., change ``` to ```text) so linters treat it as plain text; ensure the closing fence remains ``` unchanged.
516-527:⚠️ Potential issue | 🟡 MinorThe CSV patch is non-idempotent and may create duplicate environment variables.
Using JSON-patch
op: addwith path/.../env/-unconditionally appends the environment variable. If users re-run this command (e.g., after operator upgrades or by mistake), the deployment will have duplicateOTEL_EXPORTER_OTLP_ENDPOINTentries. Additionally,oc get csv ... | grepmay match multiple CSVs during upgrade windows.♻️ Suggested safer pattern
-CSV=$(oc get csv -n automotive-dev-operator-system \ - -o name | grep automotive-dev-operator) -oc patch $CSV -n automotive-dev-operator-system --type=json -p='[{ - "op": "add", - "path": "/spec/install/spec/deployments/0/spec/template/spec/containers/0/env/-", - "value": { - "name": "OTEL_EXPORTER_OTLP_ENDPOINT", - "value": "http://otel-collector.automotive-dev-operator-system.svc:4317" - } -}]' +# Get the currently-installed (Succeeded) CSV +CSV=$(oc get csv -n automotive-dev-operator-system \ + -o jsonpath='{.items[?(@.status.phase=="Succeeded")].metadata.name}' \ + | tr ' ' '\n' | grep '^automotive-dev-operator' | head -n1) + +# Only patch if OTEL_EXPORTER_OTLP_ENDPOINT is not already present +if ! oc get csv "$CSV" -n automotive-dev-operator-system \ + -o jsonpath='{.spec.install.spec.deployments[0].spec.template.spec.containers[0].env[*].name}' \ + | grep -q OTEL_EXPORTER_OTLP_ENDPOINT; then + oc patch csv "$CSV" -n automotive-dev-operator-system --type=json -p='[{ + "op": "add", + "path": "/spec/install/spec/deployments/0/spec/template/spec/containers/0/env/-", + "value": { + "name": "OTEL_EXPORTER_OTLP_ENDPOINT", + "value": "http://otel-collector.automotive-dev-operator-system.svc:4317" + } + }]' +fiNote: OLM may still revert manual CSV changes on operator upgrades. For production, consider using
OperatorConfig.spec.tracing.endpointinstead (if supported).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/observability-setup.md` around lines 516 - 527, The current JSON-patch unconditionally appends OTEL_EXPORTER_OTLP_ENDPOINT and can duplicate entries and also the grep may match multiple CSVs; instead fetch the CSV JSON for the single active CSV (e.g., pick the one with status.phase=="Succeeded" via oc get csv -n automotive-dev-operator-system -o json), use jq to idempotently remove any existing env entries named "OTEL_EXPORTER_OTLP_ENDPOINT" from .spec.install.spec.deployments[0].spec.template.spec.containers[0].env and then append the single desired env object, and finally oc apply -f - to update the CSV JSON; keep the env name OTEL_EXPORTER_OTLP_ENDPOINT and the JSON path /spec/install/spec/deployments/0/spec/template/spec/containers/0/env in your edits and prefer selecting the CSV by status or other unique field instead of a plain grep.go.mod (1)
155-160:⚠️ Potential issue | 🟠 MajorBump the OpenTelemetry modules past the flagged
otel/sdkrelease.
go.opentelemetry.io/otel/sdkis still pinned atv1.41.0, which the current scan flags forGHSA-hfvc-g4fc-pqhx. Please move the OTel modules together to a patched release so the tracing stack stays aligned.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` around lines 155 - 160, Update all OpenTelemetry module versions to a patched release that resolves GHSA-hfvc-g4fc-pqhx and keep them aligned (e.g., bump go.opentelemetry.io/otel/sdk plus the related modules go.opentelemetry.io/otel, go.opentelemetry.io/otel/exporters/otlp/otlptrace, go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc, go.opentelemetry.io/otel/metric, and go.opentelemetry.io/otel/trace) so they all use the same compatible higher version; modify the go.mod entries for those module lines to the chosen patched tag and run `go get`/`go mod tidy` to update the lock information and verify builds/tests.internal/controller/operatorconfig/controller.go (1)
312-315:⚠️ Potential issue | 🟡 Minor
status.tracingEnabledcan drift from the process’s real tracing state.This flag is derived from spec only, but tracing is initialized once at startup. After a
spec.tracing.enabledchange, status can flip even though the running operator has not reconfigured tracing yet.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/controller/operatorconfig/controller.go` around lines 312 - 315, The status field config.Status.TracingEnabled should reflect the operator's actual runtime tracing state, not just config.Spec.Tracing.Enabled; replace the current derivation (tracingEnabled := config.Spec.Tracing != nil && config.Spec.Tracing.Enabled) with a check against the runtime tracer/flag used by the operator (for example call an existing runtime accessor like tracing.IsEnabled() or check the controller's tracer/tracingInitialized boolean set by the tracing initialization routine such as SetupTracing/initializeTracing), then set config.Status.TracingEnabled based on that runtime value while still honoring spec only when the operator has actually reconfigured tracing.
🧹 Nitpick comments (1)
internal/common/telemetry/tracing.go (1)
30-37: Consider merging withresource.Default()for richer auto-detected attributes.
resource.New(ctx, resource.WithAttributes(...))only attachesservice.name. You'll typically wanttelemetry.sdk.*,process.*, host attributes, etc., plus optionallyservice.versionanddeployment.environmentso spans are easier to correlate with operator builds across environments. The idiomatic pattern is:♻️ Suggested change
- res, err := resource.New(ctx, - resource.WithAttributes( - semconv.ServiceNameKey.String(serviceName), - ), - ) + res, err := resource.New(ctx, + resource.WithFromEnv(), // OTEL_RESOURCE_ATTRIBUTES / OTEL_SERVICE_NAME + resource.WithTelemetrySDK(), + resource.WithProcess(), + resource.WithHost(), + resource.WithAttributes( + semconv.ServiceNameKey.String(serviceName), + ), + )This also lets operators override
service.name/add tags viaOTEL_RESOURCE_ATTRIBUTESwithout code changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/common/telemetry/tracing.go` around lines 30 - 37, The resource created by resource.New only sets service.name and misses auto-detected attributes; update the tracing initialization to merge the default/auto-detected resource with your custom attributes by using resource.Default (or resource.FromEnv) and resource.Merge (referencing resource.New, resource.Default, resource.Merge and semconv.ServiceNameKey) so the resulting resource contains telemetry.sdk.*, process.*, host.*, and your service.name (and optionally service.version/deployment.environment), allowing OTEL_RESOURCE_ATTRIBUTES to override or add attributes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/main.go`:
- Around line 329-351: readTracingConfig currently calls client.List on
OperatorConfigList and uses configs.Items[0], which can pick up another tenant's
config; change it to perform a direct client.Get for the specific OperatorConfig
in the operator's namespace instead of List + first-item. Locate the
readTracingConfig function and replace the List path (client.List,
OperatorConfigList, configs.Items[0]) with a Get using a NamespacedName built
from the operator's namespace and the well-known OperatorConfig name (or an
env/constant that holds that name), then read tracing settings from the
retrieved OperatorConfig (cfg.Spec.Tracing and
cfg.Spec.Tracing.GetEndpoint()/GetSamplingRatio()); ensure errors when creating
the client or when Get fails fall back to the same env/default behavior.
In `@docs/observability-setup.md`:
- Line 235: The docs use a hard-coded AWS-only storage class "gp3-csi" in the
LokiStack YAML which may not exist on other clusters; update
docs/observability-setup.md to add a note before the LokiStack YAML (referencing
the "storageClassName: gp3-csi" line and the "LokiStack" example) advising
readers to run oc get storageclass to list available storage classes and
substitute an appropriate class for their environment (e.g., managed-premium on
Azure, standard-rwo on GCP), or show a brief example command and guidance on
replacing the storageClassName value.
In `@internal/controller/imagebuild/controller.go`:
- Around line 350-352: Each phase handler (e.g., ImageBuild.HandleInitialState,
HandleUploadingState, HandleBuildingState and the other phase methods listed)
currently defers only span.End(), so replace that with a deferred closure that
checks the handler's returned error (use the named err variable), calls
span.SetStatus(codes.Error, err.Error()) and
span.SetAttributes(semconv.ErrorType(err.Error())) when err != nil, then calls
span.End(); do this in every phase handler to mark terminating errors per
OpenTelemetry conventions instead of using RecordError. Also fix the
duplicate/shadowed span variable near the HandleUploadingState →
HandleBuildingState transition by not reusing/shadowing the same span variable
(ensure the first span is ended before starting the next span or use distinct
variables) so no span is left unclosed.
---
Outside diff comments:
In `@internal/buildapi/server.go`:
- Around line 2048-2093: The GET /v1/builds/:name response construction (the
BuildResponse literal returned in the handler) currently omits the persisted
trace ID; update the BuildResponse initialization to set TraceID from the
ImageBuild annotation automotivev1alpha1.AnnotationTraceID (e.g., read
build.Annotations[automotivev1alpha1.AnnotationTraceID] and assign it to the
TraceID field) so the trace ID written at POST time is returned on subsequent
GETs; modify the BuildResponse creation in the handler that calls writeJSON to
include TraceID populated from build.Annotations.
---
Duplicate comments:
In `@docs/observability-setup.md`:
- Around line 410-411: The cleanup step currently removes "/tmp/tempo-ca.crt",
"/tmp/tempo-ca.key" and "/tmp/otel-client.*" but misses the CA serial file
created by "openssl -CAcreateserial" ("/tmp/tempo-ca.srl"); update the cleanup
command that contains the "rm /tmp/tempo-ca.crt /tmp/tempo-ca.key
/tmp/otel-client.*" invocation to also remove "/tmp/tempo-ca.srl" (or a pattern
that includes the .srl file) so no serial file is left behind.
- Around line 16-34: The fenced ASCII diagram block is missing a language
identifier causing markdownlint MD040; update the opening code fence for the
ASCII diagram (the triple backtick that begins the ASCII art block) to include
the language label "text" (i.e., change ``` to ```text) so linters treat it as
plain text; ensure the closing fence remains ``` unchanged.
- Around line 516-527: The current JSON-patch unconditionally appends
OTEL_EXPORTER_OTLP_ENDPOINT and can duplicate entries and also the grep may
match multiple CSVs; instead fetch the CSV JSON for the single active CSV (e.g.,
pick the one with status.phase=="Succeeded" via oc get csv -n
automotive-dev-operator-system -o json), use jq to idempotently remove any
existing env entries named "OTEL_EXPORTER_OTLP_ENDPOINT" from
.spec.install.spec.deployments[0].spec.template.spec.containers[0].env and then
append the single desired env object, and finally oc apply -f - to update the
CSV JSON; keep the env name OTEL_EXPORTER_OTLP_ENDPOINT and the JSON path
/spec/install/spec/deployments/0/spec/template/spec/containers/0/env in your
edits and prefer selecting the CSV by status or other unique field instead of a
plain grep.
In `@go.mod`:
- Around line 155-160: Update all OpenTelemetry module versions to a patched
release that resolves GHSA-hfvc-g4fc-pqhx and keep them aligned (e.g., bump
go.opentelemetry.io/otel/sdk plus the related modules go.opentelemetry.io/otel,
go.opentelemetry.io/otel/exporters/otlp/otlptrace,
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc,
go.opentelemetry.io/otel/metric, and go.opentelemetry.io/otel/trace) so they all
use the same compatible higher version; modify the go.mod entries for those
module lines to the chosen patched tag and run `go get`/`go mod tidy` to update
the lock information and verify builds/tests.
In `@internal/controller/operatorconfig/controller.go`:
- Around line 312-315: The status field config.Status.TracingEnabled should
reflect the operator's actual runtime tracing state, not just
config.Spec.Tracing.Enabled; replace the current derivation (tracingEnabled :=
config.Spec.Tracing != nil && config.Spec.Tracing.Enabled) with a check against
the runtime tracer/flag used by the operator (for example call an existing
runtime accessor like tracing.IsEnabled() or check the controller's
tracer/tracingInitialized boolean set by the tracing initialization routine such
as SetupTracing/initializeTracing), then set config.Status.TracingEnabled based
on that runtime value while still honoring spec only when the operator has
actually reconfigured tracing.
---
Nitpick comments:
In `@internal/common/telemetry/tracing.go`:
- Around line 30-37: The resource created by resource.New only sets service.name
and misses auto-detected attributes; update the tracing initialization to merge
the default/auto-detected resource with your custom attributes by using
resource.Default (or resource.FromEnv) and resource.Merge (referencing
resource.New, resource.Default, resource.Merge and semconv.ServiceNameKey) so
the resulting resource contains telemetry.sdk.*, process.*, host.*, and your
service.name (and optionally service.version/deployment.environment), allowing
OTEL_RESOURCE_ATTRIBUTES to override or add attributes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8e0061d3-c442-48cb-8603-f8766ad26a4d
⛔ Files ignored due to path filters (197)
config/crd/bases/automotive.sdv.cloud.redhat.com_operatorconfigs.yamlis excluded by!config/crd/bases/**go.sumis excluded by!**/*.sum,!**go.sumvendor/cel.dev/expr/BUILD.bazelis excluded by!vendor/**vendor/cel.dev/expr/MODULE.bazelis excluded by!vendor/**vendor/cel.dev/expr/checked.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/cel.dev/expr/eval.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/cel.dev/expr/explain.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/cel.dev/expr/syntax.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/cel.dev/expr/value.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/github.com/go-jose/go-jose/v4/README.mdis excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/asymmetric.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/cipher/key_wrap.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/crypter.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/shared.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/signing.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/symmetric.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/symmetric_legacy.gois excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/BUILD.bazelis excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/context.gois excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/handler.gois excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/marshal_jsonpb.gois excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/mux.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/LICENSEis excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/exporter.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform/attribute.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform/instrumentation.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform/resource.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform/span.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/LICENSEis excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/client.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/counter/counter.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/gen.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/observ/doc.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/observ/instrumentation.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/observ/target.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/partialsuccess.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/retry/retry.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/version.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/x/README.mdis excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/x/observ.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/x/x.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/version.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/LICENSEis excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/internal/x/features.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/internal/x/x.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/builtin.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/container.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/env.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_bsd.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_linux.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_readfile.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_unsupported.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_windows.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os_release_unix.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os_unix.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os_unsupported.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/process.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/resource.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/batch_span_processor.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/doc.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/id_generator.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/env/env.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/batch_span_processor.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/doc.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/simple_span_processor.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/tracer.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/provider.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/sampling.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/simple_span_processor.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/snapshot.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/span.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/span_limits.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/tracer.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/version.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/version.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.39.0/otelconv/metric.gois excluded by!vendor/**vendor/go.opentelemetry.io/proto/otlp/common/v1/common.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/go.opentelemetry.io/proto/otlp/resource/v1/resource.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/go.opentelemetry.io/proto/otlp/trace/v1/trace.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/golang.org/x/crypto/scrypt/scrypt.gois excluded by!vendor/**vendor/golang.org/x/net/html/node.gois excluded by!vendor/**vendor/golang.org/x/net/html/nodetype_string.gois excluded by!vendor/**vendor/golang.org/x/net/http2/client_priority_go126.gois excluded by!vendor/**vendor/golang.org/x/net/http2/client_priority_go127.gois excluded by!vendor/**vendor/golang.org/x/net/http2/frame.gois excluded by!vendor/**vendor/golang.org/x/net/http2/http2.gois excluded by!vendor/**vendor/golang.org/x/net/http2/server.gois excluded by!vendor/**vendor/golang.org/x/net/http2/transport.gois excluded by!vendor/**vendor/golang.org/x/net/http2/writesched_priority_rfc7540.gois excluded by!vendor/**vendor/golang.org/x/net/internal/httpsfv/httpsfv.gois excluded by!vendor/**vendor/golang.org/x/oauth2/deviceauth.gois excluded by!vendor/**vendor/golang.org/x/oauth2/oauth2.gois excluded by!vendor/**vendor/golang.org/x/oauth2/pkce.gois excluded by!vendor/**vendor/golang.org/x/oauth2/token.gois excluded by!vendor/**vendor/golang.org/x/oauth2/transport.gois excluded by!vendor/**vendor/golang.org/x/sys/cpu/cpu_arm64.gois excluded by!vendor/**vendor/golang.org/x/sys/cpu/cpu_other_arm64.gois excluded by!vendor/**vendor/golang.org/x/sys/cpu/cpu_windows_arm64.gois excluded by!vendor/**vendor/golang.org/x/sys/unix/ioctl_signed.gois excluded by!vendor/**vendor/golang.org/x/sys/unix/ioctl_unsigned.gois excluded by!vendor/**vendor/golang.org/x/sys/unix/syscall_solaris.gois excluded by!vendor/**vendor/golang.org/x/sys/unix/syscall_unix.gois excluded by!vendor/**vendor/golang.org/x/sys/windows/syscall_windows.gois excluded by!vendor/**vendor/golang.org/x/sys/windows/types_windows.gois excluded by!vendor/**vendor/golang.org/x/sys/windows/zsyscall_windows.gois excluded by!vendor/**vendor/golang.org/x/text/message/catalog/catalog.gois excluded by!vendor/**vendor/golang.org/x/text/message/catalog/dict.gois excluded by!vendor/**vendor/golang.org/x/text/message/catalog/go19.gois excluded by!vendor/**vendor/golang.org/x/text/message/catalog/gopre19.gois excluded by!vendor/**vendor/golang.org/x/text/secure/bidirule/bidirule.gois excluded by!vendor/**vendor/golang.org/x/text/secure/bidirule/bidirule10.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/secure/bidirule/bidirule9.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables10.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables11.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables12.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables13.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables15.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables17.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables9.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/forminfo.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables10.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables11.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables12.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables15.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables17.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables9.0.0.gois excluded by!vendor/**vendor/google.golang.org/grpc/CONTRIBUTING.mdis excluded by!vendor/**vendor/google.golang.org/grpc/balancer/balancer.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/pickfirst/internal/internal.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/pickfirst/pickfirst.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/pickfirst/pickfirstleaf/pickfirstleaf.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/roundrobin/roundrobin.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/subconn.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer_wrapper.gois excluded by!vendor/**vendor/google.golang.org/grpc/binarylog/grpc_binarylog_v1/binarylog.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/google.golang.org/grpc/clientconn.gois excluded by!vendor/**vendor/google.golang.org/grpc/credentials/credentials.gois excluded by!vendor/**vendor/google.golang.org/grpc/credentials/tls.gois excluded by!vendor/**vendor/google.golang.org/grpc/encoding/encoding.gois excluded by!vendor/**vendor/google.golang.org/grpc/encoding/gzip/gzip.gois excluded by!vendor/**vendor/google.golang.org/grpc/encoding/internal/internal.gois excluded by!vendor/**vendor/google.golang.org/grpc/encoding/proto/proto.gois excluded by!vendor/**vendor/google.golang.org/grpc/experimental/stats/metricregistry.gois excluded by!vendor/**vendor/google.golang.org/grpc/experimental/stats/metrics.gois excluded by!vendor/**vendor/google.golang.org/grpc/health/grpc_health_v1/health.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/google.golang.org/grpc/health/grpc_health_v1/health_grpc.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/google.golang.org/grpc/interceptor.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/balancer/gracefulswitch/gracefulswitch.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/balancer/weight/weight.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/buffer/unbounded.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/channelz/trace.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/envconfig/envconfig.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/envconfig/xds.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/experimental.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/grpcsync/callback_serializer.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/idle/idle.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/internal.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/resolver/delegatingresolver/delegatingresolver.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/resolver/dns/dns_resolver.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/stats/metrics_recorder_list.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/stats/stats.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/client_stream.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/controlbuf.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/flowcontrol.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/handler_server.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/http2_client.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/http2_server.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/http_util.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/server_stream.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/transport.gois excluded by!vendor/**vendor/google.golang.org/grpc/mem/buffer_pool.gois excluded by!vendor/**vendor/google.golang.org/grpc/mem/buffer_slice.gois excluded by!vendor/**vendor/google.golang.org/grpc/mem/buffers.gois excluded by!vendor/**vendor/google.golang.org/grpc/preloader.gois excluded by!vendor/**vendor/google.golang.org/grpc/resolver/resolver.gois excluded by!vendor/**vendor/google.golang.org/grpc/resolver_wrapper.gois excluded by!vendor/**vendor/google.golang.org/grpc/rpc_util.gois excluded by!vendor/**vendor/google.golang.org/grpc/server.gois excluded by!vendor/**vendor/google.golang.org/grpc/stream.gois excluded by!vendor/**vendor/google.golang.org/grpc/version.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/encoding/tag/tag.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/encoding/text/decode.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/filedesc/desc.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/filedesc/desc_lazy.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/genid/descriptor_gen.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/impl/codec_map.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/impl/decode.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/impl/validate.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/version/version.gois excluded by!vendor/**vendor/google.golang.org/protobuf/proto/decode.gois excluded by!vendor/**vendor/google.golang.org/protobuf/reflect/protodesc/desc.gois excluded by!vendor/**vendor/google.golang.org/protobuf/reflect/protodesc/editions.gois excluded by!vendor/**vendor/google.golang.org/protobuf/types/descriptorpb/descriptor.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/google.golang.org/protobuf/types/known/timestamppb/timestamp.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/modules.txtis excluded by!vendor/**
📒 Files selected for processing (17)
api/v1alpha1/labels.goapi/v1alpha1/operatorconfig_types.goapi/v1alpha1/zz_generated.deepcopy.gocmd/build-api/main.gocmd/main.godocs/observability-setup.mdgo.modinternal/buildapi/server.gointernal/buildapi/types.gointernal/common/tasks/tasks.gointernal/common/telemetry/tracing.gointernal/common/telemetry/tracing_test.gointernal/controller/controllerutils/labels.gointernal/controller/controllerutils/labels_test.gointernal/controller/imagebuild/controller.gointernal/controller/operatorconfig/controller.gointernal/controller/workspace/controller.go
✅ Files skipped from review due to trivial changes (3)
- internal/controller/controllerutils/labels_test.go
- api/v1alpha1/labels.go
- internal/common/tasks/tasks.go
🚧 Files skipped from review as they are similar to previous changes (3)
- internal/controller/controllerutils/labels.go
- internal/common/telemetry/tracing_test.go
- api/v1alpha1/zz_generated.deepcopy.go
bennyz
force-pushed
the
worktree-monitoring-config
branch
from
April 27, 2026 05:55
2ba35df to
8db8048
Compare
Contributor
Author
|
@ambient-code please review |
![ambient-code[bot]](https://avatars.githubusercontent.com/u/231559327?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: Initial tracing and log ingestion setup
Overall this is a well-structured PR. The OTel integration is clean, non-invasive, and degrades gracefully when tracing is disabled. Label sanitization, trace-id propagation through Tekton params/env vars, and the observability setup guide are all solid additions. Good test coverage on the new code.
A few items below for consideration — nothing blocking.
| ) | ||
| if err != nil { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
WithInsecure() is hardcoded here, which means all OTLP gRPC connections are plaintext. This is fine for an in-cluster sidecar collector (as the docs describe), but worth calling out. Consider either:
- Adding an
Insecure boolfield toTracingConfigso users can opt into TLS when the collector is remote - Or at minimum, adding a comment noting this is intentionally insecure for in-cluster use
Not blocking — the setup guide correctly deploys the collector in the same namespace, so this is safe for the documented path.
There was a problem hiding this comment.
Review: Initial Tracing and Log Ingestion Setup
Nice work — this is a clean and well-structured introduction of OpenTelemetry tracing. The approach of making tracing opt-in via OperatorConfig.Spec.Tracing, with env-var fallback, is solid. Test coverage for the new packages is good, and the SanitizeLabelValue utility + centralized buildLabels helper are welcome improvements.
A few items worth discussing inline. None are blockers.
| otlptracegrpc.WithEndpoint(endpoint), | ||
| otlptracegrpc.WithInsecure(), | ||
| ) | ||
| if err != nil { |
There was a problem hiding this comment.
The gRPC exporter is hardcoded to WithInsecure(). Fine for an initial in-cluster setup, but consider adding a // TODO: add TLS option for production/external collectors or an Insecure bool field on TracingConfig so this doesn't get forgotten.
| return id | ||
| } | ||
| if imageBuild.Annotations == nil { | ||
| imageBuild.Annotations = map[string]string{} |
There was a problem hiding this comment.
This generates a UUID (e.g. 550e8400-e29b-41d4-a716-446655440000), not an OTel trace ID (32 hex chars). The field is called trace-id but it's really a correlation ID — OTel spans in the reconcile loop will get their own actual trace IDs.
Not a blocker since it serves its purpose for cross-pod log correlation, but the naming could confuse someone debugging with Tempo who expects this to match OTel trace IDs. Maybe rename to correlation-id or add a one-line comment clarifying the distinction?
bennyz
force-pushed
the
worktree-monitoring-config
branch
from
April 27, 2026 06:34
8db8048 to
97965dd
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/buildapi/types.go (1)
244-256:⚠️ Potential issue | 🟠 MajorUpdate
internal/buildapi/openapi.yamlto match the expandedBuildResponsestruct.The OpenAPI schema only documents
name,phase,message,requestedBy, andexpiresAt, but the Go struct now includes seven additional fields:StartTime,CompletionTime,ContainerImage,DiskImage,RegistryToken,TraceID, andWarning. Generated clients will be incomplete. Additionally, per the coding guidelines for.gofiles, add failing tests before continuing with implementation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/buildapi/types.go` around lines 244 - 256, The OpenAPI spec is missing the new BuildResponse fields added to the Go struct; update internal/buildapi/openapi.yaml so the BuildResponse schema includes StartTime, CompletionTime, ContainerImage, DiskImage, RegistryToken, TraceID, and Warning (matching the JSON names startTime, completionTime, containerImage, diskImage, registryToken, traceID, warning and types as strings, with omitempty behavior preserved where applicable), then regenerate clients; also add failing tests that assert the OpenAPI schema lacks these fields before changing it (per .go coding guidelines) and then update those tests to pass once the spec is corrected so BuildResponse in the spec and the Go struct remain in sync.
♻️ Duplicate comments (4)
internal/controller/operatorconfig/controller.go (1)
312-316:⚠️ Potential issue | 🟡 Minor
status.tracingEnabledstill reflects spec, not the running tracer state.This still flips based only on
spec.tracing.enabled, even though tracing is initialized once at process start. After a spec toggle, status can report the new value before the operator pod has been restarted to apply it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/controller/operatorconfig/controller.go` around lines 312 - 316, The status field is being set from the spec via the local tracingEnabled variable (computed from config.Spec.Tracing), but it should reflect the actual running tracer state; change the logic in the block that computes tracingEnabled to query the operator's runtime tracer state (e.g., call the tracer runtime check such as tracer.IsRunning() / tracing.IsInitialized() or a similar function available in this package) and use that result instead of config.Spec.Tracing.Enabled when setting config.Status.TracingEnabled; if no runtime check is available, add a small helper (e.g., isTracerRunning()) that returns the real running state and use it so the update to config.Status.TracingEnabled only flips when the tracer process is actually running or stopped.go.mod (1)
155-160:⚠️ Potential issue | 🟡 MinorBump the OpenTelemetry stack off
v1.41.0.This is the same unresolved dependency issue from the earlier review:
go.opentelemetry.io/otel/sdk v1.41.0is still the GHSA-hfvc-g4fc-pqhx release. Because these modules are expected to move together, upgrade the whole OTel set here (otel,otel/sdk,otel/trace,otel/metric,otlptrace*) to the first patched line instead of onlysdk.Is go.opentelemetry.io/otel/sdk v1.41.0 affected by GHSA-hfvc-g4fc-pqhx, what is the first patched version, and should go.opentelemetry.io/otel, otel/sdk, otel/trace, otel/metric, and otel/exporters/otlp/otlptrace* be kept aligned when upgrading?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` around lines 155 - 160, Update all OpenTelemetry module versions so they are aligned to the first patched release that fixes GHSA-hfvc-g4fc-pqhx (do not leave only sdk bumped); change go.opentelemetry.io/otel, go.opentelemetry.io/otel/sdk, go.opentelemetry.io/otel/trace, go.opentelemetry.io/otel/metric, go.opentelemetry.io/otel/exporters/otlp/otlptrace and go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc to that same patched version (replace v1.41.0 entries with the patched version) so all OTel modules remain consistent and the vulnerability is addressed.internal/controller/imagebuild/controller.go (1)
376-381:⚠️ Potential issue | 🟠 MajorBackfill missing trace IDs before phase-specific handling.
ensureTraceIDstill only runs in the empty-phase branch. Builds that were alreadyUploading/Building/Pushing/Flashingwhen this version starts will never get the annotation, so any later TaskRun/PipelineRun created for them still carries an emptytrace-id. Move the one-time annotation/update to the top ofReconcilewhen it is absent.Suggested shape
func (r *ImageBuildReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { @@ if err := r.Get(ctx, req.NamespacedName, imageBuild); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) } + + if getTraceID(imageBuild) == "" && !isTerminalPhase(imageBuild.Status.Phase) { + ensureTraceID(ctx, imageBuild) + if err := r.Update(ctx, imageBuild); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to set trace-id annotation: %w", err) + } + return ctrl.Result{Requeue: true}, nil + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/controller/imagebuild/controller.go` around lines 376 - 381, Move the one-time trace-id backfill to the start of Reconcile: if getTraceID(imageBuild) == "" then call ensureTraceID(ctx, imageBuild) and persist it via r.Update(ctx, imageBuild) before any phase-specific branches so builds already in Uploading/Building/Pushing/Flashing get annotated; keep the existing error handling (wrap with fmt.Errorf("failed to set trace-id annotation: %w")) and ensure this logic runs unconditionally at Reconcile entry rather than inside the empty-phase branch.cmd/main.go (1)
214-218:⚠️ Potential issue | 🟠 MajorBound tracing shutdown with a timeout.
shutdownTracing(context.Background())can block while flushing spans. On pod termination that can overrun the grace window or hang shutdown entirely.Suggested fix
+ "time" @@ defer func() { - if err := shutdownTracing(context.Background()); err != nil { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + if err := shutdownTracing(ctx); err != nil { setupLog.Error(err, "failed to shutdown tracing") } }()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/main.go` around lines 214 - 218, The shutdownTracing call should use a bounded context so flushing can't hang shutdown: replace shutdownTracing(context.Background()) with a context created via context.WithTimeout (e.g., ctx, cancel := context.WithTimeout(context.Background(), <reasonableDuration>)) and defer cancel() before calling shutdownTracing(ctx); propagate any error from shutdownTracing(ctx) into setupLog.Error as currently done (shutdownTracing, setupLog.Error, cancel are the symbols to change). Ensure the timeout duration is reasonable for span flush (not zero) and that cancel() is deferred to release resources.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/v1alpha1/zz_generated.deepcopy.go`:
- Around line 1193-1197: Revert the changes to the generated file
api/v1alpha1/zz_generated.deepcopy.go (including the Tracing DeepCopyInto block
and the other generated blocks around lines 1299-1317) so the PR only contains
the source type change; do not commit controller-gen output. Remove
zz_generated.deepcopy.go from the staged changes (or reset that file) and ensure
generated deepcopy and CRD manifest artifacts are excluded from commits going
forward by adding/confirming an appropriate ignore or documenting generation in
the repo’s CONTRIBUTING/Makefile so maintainers regenerate them during CI rather
than committing them.
In `@cmd/main.go`:
- Around line 208-209: The startup code is incorrectly passing WATCH_NAMESPACE
into readTracingConfig (and similar calls around lines 329-348), causing
OperatorConfig lookups to use an empty namespace for cluster-scoped deployments;
update the call sites to derive the operator pod namespace (e.g., from the
POD_NAMESPACE env var or the serviceaccount namespace file at
/var/run/secrets/kubernetes.io/serviceaccount/namespace) and pass that
operatorNamespace into readTracingConfig and any other OperatorConfig reads
instead of watchNamespace so spec.tracing is honored at startup (adjust
callsites referencing readTracingConfig, telemetry.InitTracing, and any
OperatorConfig Get{Name: "config", Namespace: ...} lookups).
In `@docs/observability-setup.md`:
- Around line 612-625: Update the LogQL examples to use the operator namespace
and the new trace ID field: replace occurrences of
kubernetes_namespace_name="automotive-dev-operator-builds" with
kubernetes_namespace_name="automotive-dev-operator-system", and where the
queries look for a uuid or build-specific pod labels, instead filter by the
controller-emitted 32-hex trace ID value (the controller now writes a 32-hex
trace ID into logs) and by kubernetes_container_name="manager" / msg != "" as
appropriate; apply the same corrections to the other example block (previously
lines ~643-652) so all verification queries target the operator namespace and
the trace-id format.
In `@internal/buildapi/server.go`:
- Around line 1688-1691: The createBuild handler starts a span (ctx, span :=
apiTracer.Start(...)) but never records errors on early returns; update every
early-return error path (JSON binding, request validation, apply defaults,
kubernetes client retrieval, TTL resolution, extra repos resolution, secure
build validation, workspace resolution, existing build checks, registry
resolution, secret creation, ImageBuild creation) to call span.RecordError(err)
and span.SetStatus(codes.Error, err.Error()) immediately before writing the HTTP
error response/return; ensure codes is imported from the OpenTelemetry package
and reuse the existing span variable so failed build submissions are marked as
errors in traces.
In `@internal/common/telemetry/tracing.go`:
- Around line 24-25: The endpoint handling in tracing.go currently reads
OTEL_EXPORTER_OTLP_ENDPOINT and passes it to otlptracegrpc.WithEndpoint (which
requires host:port), so URLs with a scheme will break; update the code paths
that set the exporter options (the places using otlptracegrpc.WithEndpoint — see
the usage around the endpoint variable and again at the block referenced by
lines 40-42) to call otlptracegrpc.WithEndpointURL when the value is a full URL
(or alternatively strip the scheme before calling WithEndpoint), and ensure you
only call WithEndpoint/WithEndpointURL appropriately depending on whether the
env var contains a full URL; also add a unit/integration test that sets
OTEL_EXPORTER_OTLP_ENDPOINT to a URL with scheme (e.g., http://localhost:4317)
and asserts exporter construction succeeds or the correct option is used.
In `@internal/controller/controllerutils/tracing.go`:
- Around line 10-15: EndSpanWithError currently dereferences the pointer err
without checking for nil which can panic if callers pass a nil pointer; update
the function (EndSpanWithError(span trace.Span, err *error)) to first check if
err != nil && *err != nil before calling span.RecordError and span.SetStatus,
and always call span.End() afterward so the span is closed even when err is nil
or the pointer is nil.
---
Outside diff comments:
In `@internal/buildapi/types.go`:
- Around line 244-256: The OpenAPI spec is missing the new BuildResponse fields
added to the Go struct; update internal/buildapi/openapi.yaml so the
BuildResponse schema includes StartTime, CompletionTime, ContainerImage,
DiskImage, RegistryToken, TraceID, and Warning (matching the JSON names
startTime, completionTime, containerImage, diskImage, registryToken, traceID,
warning and types as strings, with omitempty behavior preserved where
applicable), then regenerate clients; also add failing tests that assert the
OpenAPI schema lacks these fields before changing it (per .go coding guidelines)
and then update those tests to pass once the spec is corrected so BuildResponse
in the spec and the Go struct remain in sync.
---
Duplicate comments:
In `@cmd/main.go`:
- Around line 214-218: The shutdownTracing call should use a bounded context so
flushing can't hang shutdown: replace shutdownTracing(context.Background()) with
a context created via context.WithTimeout (e.g., ctx, cancel :=
context.WithTimeout(context.Background(), <reasonableDuration>)) and defer
cancel() before calling shutdownTracing(ctx); propagate any error from
shutdownTracing(ctx) into setupLog.Error as currently done (shutdownTracing,
setupLog.Error, cancel are the symbols to change). Ensure the timeout duration
is reasonable for span flush (not zero) and that cancel() is deferred to release
resources.
In `@go.mod`:
- Around line 155-160: Update all OpenTelemetry module versions so they are
aligned to the first patched release that fixes GHSA-hfvc-g4fc-pqhx (do not
leave only sdk bumped); change go.opentelemetry.io/otel,
go.opentelemetry.io/otel/sdk, go.opentelemetry.io/otel/trace,
go.opentelemetry.io/otel/metric,
go.opentelemetry.io/otel/exporters/otlp/otlptrace and
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc to that same
patched version (replace v1.41.0 entries with the patched version) so all OTel
modules remain consistent and the vulnerability is addressed.
In `@internal/controller/imagebuild/controller.go`:
- Around line 376-381: Move the one-time trace-id backfill to the start of
Reconcile: if getTraceID(imageBuild) == "" then call ensureTraceID(ctx,
imageBuild) and persist it via r.Update(ctx, imageBuild) before any
phase-specific branches so builds already in Uploading/Building/Pushing/Flashing
get annotated; keep the existing error handling (wrap with fmt.Errorf("failed to
set trace-id annotation: %w")) and ensure this logic runs unconditionally at
Reconcile entry rather than inside the empty-phase branch.
In `@internal/controller/operatorconfig/controller.go`:
- Around line 312-316: The status field is being set from the spec via the local
tracingEnabled variable (computed from config.Spec.Tracing), but it should
reflect the actual running tracer state; change the logic in the block that
computes tracingEnabled to query the operator's runtime tracer state (e.g., call
the tracer runtime check such as tracer.IsRunning() / tracing.IsInitialized() or
a similar function available in this package) and use that result instead of
config.Spec.Tracing.Enabled when setting config.Status.TracingEnabled; if no
runtime check is available, add a small helper (e.g., isTracerRunning()) that
returns the real running state and use it so the update to
config.Status.TracingEnabled only flips when the tracer process is actually
running or stopped.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7687c0e0-56d8-41e3-97d5-3f42d338ca06
⛔ Files ignored due to path filters (197)
config/crd/bases/automotive.sdv.cloud.redhat.com_operatorconfigs.yamlis excluded by!config/crd/bases/**go.sumis excluded by!**/*.sum,!**go.sumvendor/cel.dev/expr/BUILD.bazelis excluded by!vendor/**vendor/cel.dev/expr/MODULE.bazelis excluded by!vendor/**vendor/cel.dev/expr/checked.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/cel.dev/expr/eval.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/cel.dev/expr/explain.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/cel.dev/expr/syntax.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/cel.dev/expr/value.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/github.com/go-jose/go-jose/v4/README.mdis excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/asymmetric.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/cipher/key_wrap.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/crypter.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/shared.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/signing.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/symmetric.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/symmetric_legacy.gois excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/BUILD.bazelis excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/context.gois excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/handler.gois excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/marshal_jsonpb.gois excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/mux.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/LICENSEis excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/exporter.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform/attribute.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform/instrumentation.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform/resource.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform/span.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/LICENSEis excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/client.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/counter/counter.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/gen.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/observ/doc.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/observ/instrumentation.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/observ/target.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/partialsuccess.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/retry/retry.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/version.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/x/README.mdis excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/x/observ.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/x/x.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/version.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/LICENSEis excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/internal/x/features.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/internal/x/x.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/builtin.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/container.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/env.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_bsd.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_linux.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_readfile.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_unsupported.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_windows.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os_release_unix.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os_unix.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os_unsupported.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/process.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/resource.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/batch_span_processor.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/doc.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/id_generator.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/env/env.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/batch_span_processor.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/doc.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/simple_span_processor.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/tracer.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/provider.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/sampling.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/simple_span_processor.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/snapshot.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/span.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/span_limits.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/tracer.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/version.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/version.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.39.0/otelconv/metric.gois excluded by!vendor/**vendor/go.opentelemetry.io/proto/otlp/common/v1/common.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/go.opentelemetry.io/proto/otlp/resource/v1/resource.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/go.opentelemetry.io/proto/otlp/trace/v1/trace.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/golang.org/x/crypto/scrypt/scrypt.gois excluded by!vendor/**vendor/golang.org/x/net/html/node.gois excluded by!vendor/**vendor/golang.org/x/net/html/nodetype_string.gois excluded by!vendor/**vendor/golang.org/x/net/http2/client_priority_go126.gois excluded by!vendor/**vendor/golang.org/x/net/http2/client_priority_go127.gois excluded by!vendor/**vendor/golang.org/x/net/http2/frame.gois excluded by!vendor/**vendor/golang.org/x/net/http2/http2.gois excluded by!vendor/**vendor/golang.org/x/net/http2/server.gois excluded by!vendor/**vendor/golang.org/x/net/http2/transport.gois excluded by!vendor/**vendor/golang.org/x/net/http2/writesched_priority_rfc7540.gois excluded by!vendor/**vendor/golang.org/x/net/internal/httpsfv/httpsfv.gois excluded by!vendor/**vendor/golang.org/x/oauth2/deviceauth.gois excluded by!vendor/**vendor/golang.org/x/oauth2/oauth2.gois excluded by!vendor/**vendor/golang.org/x/oauth2/pkce.gois excluded by!vendor/**vendor/golang.org/x/oauth2/token.gois excluded by!vendor/**vendor/golang.org/x/oauth2/transport.gois excluded by!vendor/**vendor/golang.org/x/sys/cpu/cpu_arm64.gois excluded by!vendor/**vendor/golang.org/x/sys/cpu/cpu_other_arm64.gois excluded by!vendor/**vendor/golang.org/x/sys/cpu/cpu_windows_arm64.gois excluded by!vendor/**vendor/golang.org/x/sys/unix/ioctl_signed.gois excluded by!vendor/**vendor/golang.org/x/sys/unix/ioctl_unsigned.gois excluded by!vendor/**vendor/golang.org/x/sys/unix/syscall_solaris.gois excluded by!vendor/**vendor/golang.org/x/sys/unix/syscall_unix.gois excluded by!vendor/**vendor/golang.org/x/sys/windows/syscall_windows.gois excluded by!vendor/**vendor/golang.org/x/sys/windows/types_windows.gois excluded by!vendor/**vendor/golang.org/x/sys/windows/zsyscall_windows.gois excluded by!vendor/**vendor/golang.org/x/text/message/catalog/catalog.gois excluded by!vendor/**vendor/golang.org/x/text/message/catalog/dict.gois excluded by!vendor/**vendor/golang.org/x/text/message/catalog/go19.gois excluded by!vendor/**vendor/golang.org/x/text/message/catalog/gopre19.gois excluded by!vendor/**vendor/golang.org/x/text/secure/bidirule/bidirule.gois excluded by!vendor/**vendor/golang.org/x/text/secure/bidirule/bidirule10.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/secure/bidirule/bidirule9.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables10.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables11.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables12.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables13.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables15.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables17.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables9.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/forminfo.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables10.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables11.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables12.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables15.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables17.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables9.0.0.gois excluded by!vendor/**vendor/google.golang.org/grpc/CONTRIBUTING.mdis excluded by!vendor/**vendor/google.golang.org/grpc/balancer/balancer.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/pickfirst/internal/internal.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/pickfirst/pickfirst.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/pickfirst/pickfirstleaf/pickfirstleaf.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/roundrobin/roundrobin.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/subconn.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer_wrapper.gois excluded by!vendor/**vendor/google.golang.org/grpc/binarylog/grpc_binarylog_v1/binarylog.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/google.golang.org/grpc/clientconn.gois excluded by!vendor/**vendor/google.golang.org/grpc/credentials/credentials.gois excluded by!vendor/**vendor/google.golang.org/grpc/credentials/tls.gois excluded by!vendor/**vendor/google.golang.org/grpc/encoding/encoding.gois excluded by!vendor/**vendor/google.golang.org/grpc/encoding/gzip/gzip.gois excluded by!vendor/**vendor/google.golang.org/grpc/encoding/internal/internal.gois excluded by!vendor/**vendor/google.golang.org/grpc/encoding/proto/proto.gois excluded by!vendor/**vendor/google.golang.org/grpc/experimental/stats/metricregistry.gois excluded by!vendor/**vendor/google.golang.org/grpc/experimental/stats/metrics.gois excluded by!vendor/**vendor/google.golang.org/grpc/health/grpc_health_v1/health.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/google.golang.org/grpc/health/grpc_health_v1/health_grpc.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/google.golang.org/grpc/interceptor.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/balancer/gracefulswitch/gracefulswitch.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/balancer/weight/weight.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/buffer/unbounded.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/channelz/trace.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/envconfig/envconfig.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/envconfig/xds.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/experimental.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/grpcsync/callback_serializer.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/idle/idle.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/internal.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/resolver/delegatingresolver/delegatingresolver.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/resolver/dns/dns_resolver.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/stats/metrics_recorder_list.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/stats/stats.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/client_stream.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/controlbuf.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/flowcontrol.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/handler_server.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/http2_client.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/http2_server.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/http_util.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/server_stream.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/transport.gois excluded by!vendor/**vendor/google.golang.org/grpc/mem/buffer_pool.gois excluded by!vendor/**vendor/google.golang.org/grpc/mem/buffer_slice.gois excluded by!vendor/**vendor/google.golang.org/grpc/mem/buffers.gois excluded by!vendor/**vendor/google.golang.org/grpc/preloader.gois excluded by!vendor/**vendor/google.golang.org/grpc/resolver/resolver.gois excluded by!vendor/**vendor/google.golang.org/grpc/resolver_wrapper.gois excluded by!vendor/**vendor/google.golang.org/grpc/rpc_util.gois excluded by!vendor/**vendor/google.golang.org/grpc/server.gois excluded by!vendor/**vendor/google.golang.org/grpc/stream.gois excluded by!vendor/**vendor/google.golang.org/grpc/version.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/encoding/tag/tag.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/encoding/text/decode.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/filedesc/desc.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/filedesc/desc_lazy.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/genid/descriptor_gen.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/impl/codec_map.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/impl/decode.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/impl/validate.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/version/version.gois excluded by!vendor/**vendor/google.golang.org/protobuf/proto/decode.gois excluded by!vendor/**vendor/google.golang.org/protobuf/reflect/protodesc/desc.gois excluded by!vendor/**vendor/google.golang.org/protobuf/reflect/protodesc/editions.gois excluded by!vendor/**vendor/google.golang.org/protobuf/types/descriptorpb/descriptor.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/google.golang.org/protobuf/types/known/timestamppb/timestamp.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/modules.txtis excluded by!vendor/**
📒 Files selected for processing (19)
api/v1alpha1/labels.goapi/v1alpha1/operatorconfig_types.goapi/v1alpha1/zz_generated.deepcopy.gocmd/build-api/main.gocmd/main.godocs/observability-setup.mdgo.modinternal/buildapi/server.gointernal/buildapi/types.gointernal/common/tasks/tasks.gointernal/common/tasks/traceid_test.gointernal/common/telemetry/tracing.gointernal/common/telemetry/tracing_test.gointernal/controller/controllerutils/labels.gointernal/controller/controllerutils/labels_test.gointernal/controller/controllerutils/tracing.gointernal/controller/imagebuild/controller.gointernal/controller/operatorconfig/controller.gointernal/controller/workspace/controller.go
✅ Files skipped from review due to trivial changes (1)
- api/v1alpha1/labels.go
🚧 Files skipped from review as they are similar to previous changes (5)
- internal/controller/controllerutils/labels.go
- internal/controller/controllerutils/labels_test.go
- internal/common/telemetry/tracing_test.go
- internal/controller/workspace/controller.go
- cmd/build-api/main.go
bennyz
force-pushed
the
worktree-monitoring-config
branch
2 times, most recently
from
April 27, 2026 07:08
16ee7c8 to
89a18e2
Compare
bennyz
force-pushed
the
worktree-monitoring-config
branch
from
April 27, 2026 08:33
89a18e2 to
64798fd
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
internal/controller/imagebuild/controller.go (1)
376-381:⚠️ Potential issue | 🟡 MinorBackfill
trace-idoutside the initial-state path too.This still only annotates brand-new builds. Any
ImageBuildalready inUploading/Building/Pushing/Flashingduring an upgrade never reacheshandleInitialState, so the downstreamtrace-idparams/labels stay empty for the rest of that build.Suggested direction
func (r *ImageBuildReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { @@ imageBuild := &automotivev1alpha1.ImageBuild{} if err := r.Get(ctx, req.NamespacedName, imageBuild); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) } + + if getTraceID(imageBuild) == "" && !isTerminalPhase(imageBuild.Status.Phase) { + ensureTraceID(ctx, imageBuild) + if err := r.Update(ctx, imageBuild); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to set trace-id annotation: %w", err) + } + return ctrl.Result{Requeue: true}, nil + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/controller/imagebuild/controller.go` around lines 376 - 381, Currently getTraceID/ensureTraceID is only invoked in the initial-state path so existing ImageBuilds in Uploading/Building/Pushing/Flashing never get backfilled; move or add a trace-id backfill early in the reconcile flow (before state-specific handlers like handleInitialState) so every reconcile checks if getTraceID(imageBuild) == "" then calls ensureTraceID(ctx, imageBuild) and persists via r.Update(ctx, imageBuild) (handle and return any update error as done now). Update the reconcile function to call this logic unconditionally (or at least for non-terminal states) so trace-id is populated for in-flight builds as well.
🧹 Nitpick comments (2)
internal/controller/operatorconfig/controller.go (1)
323-326:reconcileMonitoringreturn value is discarded.The bool indicating whether
Status.MonitoringEnabledchanged is dropped, but it's the only purpose of returning it. Since the status update at line 339 runs unconditionally, the helper can simply returnerror. Minor cleanup.Suggested simplification
- if _, err := r.reconcileMonitoring(ctx, config); err != nil { + if err := r.reconcileMonitoring(ctx, config); err != nil { return ctrl.Result{}, err }…and drop the
boolfromreconcileMonitoring's signature.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/controller/operatorconfig/controller.go` around lines 323 - 326, The call to reconcileMonitoring currently discards its bool return (which indicates whether Status.MonitoringEnabled changed) so simplify by removing that unused bool: change reconcileMonitoring's signature to return only error, update its implementation to drop the bool return value, and update this call site in reconcile (and any other callers) to only check the error (e.g., if err := r.reconcileMonitoring(ctx, config); err != nil { return ctrl.Result{}, err }), leaving the unconditional status update intact; also remove any internal logic that relies on returning the bool.internal/common/telemetry/tracing_test.go (1)
23-44:TestInitTracing_NoEndpointandTestInitTracing_NoEndpoint_ReturnsNoopProviderlargely overlap.Both clear the env var, call
InitTracingwith empty endpoint, shut down, and assert the global provider behaves as a no-op (one checksIsValid(), the other checksTraceID()is zero — equivalent). Consider folding into a single test or extending one with both assertions to keep the suite tight.Also applies to: 94-115
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/common/telemetry/tracing_test.go` around lines 23 - 44, Tests TestInitTracing_NoEndpoint and TestInitTracing_NoEndpoint_ReturnsNoopProvider duplicate setup and assertions; consolidate them by keeping one test (e.g., TestInitTracing_NoEndpoint) that calls saveAndRestoreOTelGlobals, clears OTEL_EXPORTER_OTLP_ENDPOINT, invokes InitTracing(...), calls shutdown, and then performs both checks (span.SpanContext().IsValid() is false and the span's TraceID is zero) using the same tracer obtained via otel.GetTracerProvider() and tp.Tracer("test"); remove the redundant test to avoid overlap.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/buildapi/openapi.yaml`:
- Around line 253-255: Update the JSON property casing for acronym fields to use
camelCase consistently: change the OpenAPI property name traceID to traceId and
leaseId accordingly, and ensure the corresponding Go struct fields TraceID and
LeaseID have matching struct tags json:"traceId,omitempty" and
json:"leaseId,omitempty" (update any related OpenAPI definitions and Go struct
tags so TraceID and LeaseID are consistently serialized as traceId and leaseId).
In `@internal/buildapi/server.go`:
- Around line 63-69: extractTraceID currently returns a fresh random ID on each
call when no span exists, causing different handlers to see different fallback
IDs; change this by generating a single stable fallback trace ID once in request
middleware (e.g., a new middleware function or the existing request setup) and
storing it in the request context under a private context key, then modify
extractTraceID to first check for an existing span
(trace.SpanContextFromContext) and if none, read and return the stored fallback
ID from the context (only generate a new random ID inside the middleware when
initializing the context value). Ensure the context key is unexported and used
by both your middleware and extractTraceID so all handlers (including where
ImageBuild annotations and request logs are written) see the same ID for the
lifetime of the request.
In `@internal/common/telemetry/tracing_test.go`:
- Around line 117-130: The test TestInitTracing_PropagatorSet currently discards
the shutdown function returned by InitTracing, leaking the tracer provider and
OTLP resources; change the call to capture the shutdown (e.g., shutdown, err :=
InitTracing(...)) and immediately defer calling it with a context (e.g., defer
shutdown(context.Background()) or a short timeout context) so the batch span
processor and gRPC connection are cleaned up after the test; reference
InitTracing and the returned shutdown function in your change.
In `@internal/controller/workspace/controller.go`:
- Around line 453-455: Compute and check the sanitized owner value before
writing the label: call controllerutils.SanitizeLabelValue(ws.Spec.Owner) into a
local (e.g., sanitizedOwner) and only set labels[automotivev1alpha1.LabelOwner]
= sanitizedOwner when sanitizedOwner != ""; replace the current guard that
checks ws.Spec.Owner != "" so you don't write an empty label when sanitization
returns an empty string.
---
Duplicate comments:
In `@internal/controller/imagebuild/controller.go`:
- Around line 376-381: Currently getTraceID/ensureTraceID is only invoked in the
initial-state path so existing ImageBuilds in
Uploading/Building/Pushing/Flashing never get backfilled; move or add a trace-id
backfill early in the reconcile flow (before state-specific handlers like
handleInitialState) so every reconcile checks if getTraceID(imageBuild) == ""
then calls ensureTraceID(ctx, imageBuild) and persists via r.Update(ctx,
imageBuild) (handle and return any update error as done now). Update the
reconcile function to call this logic unconditionally (or at least for
non-terminal states) so trace-id is populated for in-flight builds as well.
---
Nitpick comments:
In `@internal/common/telemetry/tracing_test.go`:
- Around line 23-44: Tests TestInitTracing_NoEndpoint and
TestInitTracing_NoEndpoint_ReturnsNoopProvider duplicate setup and assertions;
consolidate them by keeping one test (e.g., TestInitTracing_NoEndpoint) that
calls saveAndRestoreOTelGlobals, clears OTEL_EXPORTER_OTLP_ENDPOINT, invokes
InitTracing(...), calls shutdown, and then performs both checks
(span.SpanContext().IsValid() is false and the span's TraceID is zero) using the
same tracer obtained via otel.GetTracerProvider() and tp.Tracer("test"); remove
the redundant test to avoid overlap.
In `@internal/controller/operatorconfig/controller.go`:
- Around line 323-326: The call to reconcileMonitoring currently discards its
bool return (which indicates whether Status.MonitoringEnabled changed) so
simplify by removing that unused bool: change reconcileMonitoring's signature to
return only error, update its implementation to drop the bool return value, and
update this call site in reconcile (and any other callers) to only check the
error (e.g., if err := r.reconcileMonitoring(ctx, config); err != nil { return
ctrl.Result{}, err }), leaving the unconditional status update intact; also
remove any internal logic that relies on returning the bool.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d279a71c-18c6-4617-8e45-b25474686a7f
⛔ Files ignored due to path filters (197)
config/crd/bases/automotive.sdv.cloud.redhat.com_operatorconfigs.yamlis excluded by!config/crd/bases/**go.sumis excluded by!**/*.sum,!**go.sumvendor/cel.dev/expr/BUILD.bazelis excluded by!vendor/**vendor/cel.dev/expr/MODULE.bazelis excluded by!vendor/**vendor/cel.dev/expr/checked.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/cel.dev/expr/eval.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/cel.dev/expr/explain.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/cel.dev/expr/syntax.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/cel.dev/expr/value.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/github.com/go-jose/go-jose/v4/README.mdis excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/asymmetric.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/cipher/key_wrap.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/crypter.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/shared.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/signing.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/symmetric.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/symmetric_legacy.gois excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/BUILD.bazelis excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/context.gois excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/handler.gois excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/marshal_jsonpb.gois excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/mux.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/LICENSEis excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/exporter.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform/attribute.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform/instrumentation.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform/resource.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform/span.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/LICENSEis excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/client.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/counter/counter.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/gen.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/observ/doc.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/observ/instrumentation.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/observ/target.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/partialsuccess.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/retry/retry.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/version.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/x/README.mdis excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/x/observ.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/x/x.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/version.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/LICENSEis excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/internal/x/features.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/internal/x/x.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/builtin.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/container.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/env.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_bsd.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_linux.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_readfile.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_unsupported.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_windows.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os_release_unix.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os_unix.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os_unsupported.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/process.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/resource.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/batch_span_processor.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/doc.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/id_generator.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/env/env.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/batch_span_processor.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/doc.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/simple_span_processor.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/tracer.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/provider.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/sampling.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/simple_span_processor.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/snapshot.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/span.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/span_limits.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/tracer.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/version.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/version.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.39.0/otelconv/metric.gois excluded by!vendor/**vendor/go.opentelemetry.io/proto/otlp/common/v1/common.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/go.opentelemetry.io/proto/otlp/resource/v1/resource.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/go.opentelemetry.io/proto/otlp/trace/v1/trace.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/golang.org/x/crypto/scrypt/scrypt.gois excluded by!vendor/**vendor/golang.org/x/net/html/node.gois excluded by!vendor/**vendor/golang.org/x/net/html/nodetype_string.gois excluded by!vendor/**vendor/golang.org/x/net/http2/client_priority_go126.gois excluded by!vendor/**vendor/golang.org/x/net/http2/client_priority_go127.gois excluded by!vendor/**vendor/golang.org/x/net/http2/frame.gois excluded by!vendor/**vendor/golang.org/x/net/http2/http2.gois excluded by!vendor/**vendor/golang.org/x/net/http2/server.gois excluded by!vendor/**vendor/golang.org/x/net/http2/transport.gois excluded by!vendor/**vendor/golang.org/x/net/http2/writesched_priority_rfc7540.gois excluded by!vendor/**vendor/golang.org/x/net/internal/httpsfv/httpsfv.gois excluded by!vendor/**vendor/golang.org/x/oauth2/deviceauth.gois excluded by!vendor/**vendor/golang.org/x/oauth2/oauth2.gois excluded by!vendor/**vendor/golang.org/x/oauth2/pkce.gois excluded by!vendor/**vendor/golang.org/x/oauth2/token.gois excluded by!vendor/**vendor/golang.org/x/oauth2/transport.gois excluded by!vendor/**vendor/golang.org/x/sys/cpu/cpu_arm64.gois excluded by!vendor/**vendor/golang.org/x/sys/cpu/cpu_other_arm64.gois excluded by!vendor/**vendor/golang.org/x/sys/cpu/cpu_windows_arm64.gois excluded by!vendor/**vendor/golang.org/x/sys/unix/ioctl_signed.gois excluded by!vendor/**vendor/golang.org/x/sys/unix/ioctl_unsigned.gois excluded by!vendor/**vendor/golang.org/x/sys/unix/syscall_solaris.gois excluded by!vendor/**vendor/golang.org/x/sys/unix/syscall_unix.gois excluded by!vendor/**vendor/golang.org/x/sys/windows/syscall_windows.gois excluded by!vendor/**vendor/golang.org/x/sys/windows/types_windows.gois excluded by!vendor/**vendor/golang.org/x/sys/windows/zsyscall_windows.gois excluded by!vendor/**vendor/golang.org/x/text/message/catalog/catalog.gois excluded by!vendor/**vendor/golang.org/x/text/message/catalog/dict.gois excluded by!vendor/**vendor/golang.org/x/text/message/catalog/go19.gois excluded by!vendor/**vendor/golang.org/x/text/message/catalog/gopre19.gois excluded by!vendor/**vendor/golang.org/x/text/secure/bidirule/bidirule.gois excluded by!vendor/**vendor/golang.org/x/text/secure/bidirule/bidirule10.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/secure/bidirule/bidirule9.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables10.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables11.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables12.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables13.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables15.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables17.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables9.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/forminfo.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables10.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables11.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables12.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables15.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables17.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables9.0.0.gois excluded by!vendor/**vendor/google.golang.org/grpc/CONTRIBUTING.mdis excluded by!vendor/**vendor/google.golang.org/grpc/balancer/balancer.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/pickfirst/internal/internal.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/pickfirst/pickfirst.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/pickfirst/pickfirstleaf/pickfirstleaf.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/roundrobin/roundrobin.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/subconn.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer_wrapper.gois excluded by!vendor/**vendor/google.golang.org/grpc/binarylog/grpc_binarylog_v1/binarylog.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/google.golang.org/grpc/clientconn.gois excluded by!vendor/**vendor/google.golang.org/grpc/credentials/credentials.gois excluded by!vendor/**vendor/google.golang.org/grpc/credentials/tls.gois excluded by!vendor/**vendor/google.golang.org/grpc/encoding/encoding.gois excluded by!vendor/**vendor/google.golang.org/grpc/encoding/gzip/gzip.gois excluded by!vendor/**vendor/google.golang.org/grpc/encoding/internal/internal.gois excluded by!vendor/**vendor/google.golang.org/grpc/encoding/proto/proto.gois excluded by!vendor/**vendor/google.golang.org/grpc/experimental/stats/metricregistry.gois excluded by!vendor/**vendor/google.golang.org/grpc/experimental/stats/metrics.gois excluded by!vendor/**vendor/google.golang.org/grpc/health/grpc_health_v1/health.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/google.golang.org/grpc/health/grpc_health_v1/health_grpc.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/google.golang.org/grpc/interceptor.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/balancer/gracefulswitch/gracefulswitch.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/balancer/weight/weight.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/buffer/unbounded.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/channelz/trace.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/envconfig/envconfig.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/envconfig/xds.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/experimental.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/grpcsync/callback_serializer.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/idle/idle.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/internal.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/resolver/delegatingresolver/delegatingresolver.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/resolver/dns/dns_resolver.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/stats/metrics_recorder_list.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/stats/stats.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/client_stream.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/controlbuf.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/flowcontrol.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/handler_server.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/http2_client.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/http2_server.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/http_util.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/server_stream.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/transport.gois excluded by!vendor/**vendor/google.golang.org/grpc/mem/buffer_pool.gois excluded by!vendor/**vendor/google.golang.org/grpc/mem/buffer_slice.gois excluded by!vendor/**vendor/google.golang.org/grpc/mem/buffers.gois excluded by!vendor/**vendor/google.golang.org/grpc/preloader.gois excluded by!vendor/**vendor/google.golang.org/grpc/resolver/resolver.gois excluded by!vendor/**vendor/google.golang.org/grpc/resolver_wrapper.gois excluded by!vendor/**vendor/google.golang.org/grpc/rpc_util.gois excluded by!vendor/**vendor/google.golang.org/grpc/server.gois excluded by!vendor/**vendor/google.golang.org/grpc/stream.gois excluded by!vendor/**vendor/google.golang.org/grpc/version.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/encoding/tag/tag.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/encoding/text/decode.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/filedesc/desc.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/filedesc/desc_lazy.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/genid/descriptor_gen.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/impl/codec_map.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/impl/decode.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/impl/validate.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/version/version.gois excluded by!vendor/**vendor/google.golang.org/protobuf/proto/decode.gois excluded by!vendor/**vendor/google.golang.org/protobuf/reflect/protodesc/desc.gois excluded by!vendor/**vendor/google.golang.org/protobuf/reflect/protodesc/editions.gois excluded by!vendor/**vendor/google.golang.org/protobuf/types/descriptorpb/descriptor.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/google.golang.org/protobuf/types/known/timestamppb/timestamp.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/modules.txtis excluded by!vendor/**
📒 Files selected for processing (20)
api/v1alpha1/labels.goapi/v1alpha1/operatorconfig_types.goapi/v1alpha1/zz_generated.deepcopy.gocmd/build-api/main.gocmd/main.godocs/observability-setup.mdgo.modinternal/buildapi/openapi.yamlinternal/buildapi/server.gointernal/buildapi/types.gointernal/common/tasks/tasks.gointernal/common/tasks/traceid_test.gointernal/common/telemetry/tracing.gointernal/common/telemetry/tracing_test.gointernal/controller/controllerutils/labels.gointernal/controller/controllerutils/labels_test.gointernal/controller/controllerutils/tracing.gointernal/controller/imagebuild/controller.gointernal/controller/operatorconfig/controller.gointernal/controller/workspace/controller.go
✅ Files skipped from review due to trivial changes (5)
- internal/buildapi/types.go
- internal/common/tasks/traceid_test.go
- internal/controller/controllerutils/labels_test.go
- api/v1alpha1/labels.go
- internal/common/tasks/tasks.go
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/controller/controllerutils/tracing.go
bkhizgiy
reviewed
Apr 27, 2026
bennyz
force-pushed
the
worktree-monitoring-config
branch
from
April 27, 2026 10:25
64798fd to
af48c3d
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
docs/observability-setup.md (1)
234-234:⚠️ Potential issue | 🟡 MinorDocument storage class portability for
efs-sc.Line 234 hard-codes an environment-specific storage class. Add a short note to list available classes (
oc get storageclass) and substitute per cluster.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/observability-setup.md` at line 234, The docs currently hard-code storageClassName: efs-sc; update the "storageClassName" entry to mention that "efs-sc" is environment-specific, add a short note instructing readers to run `oc get storageclass` to list available classes and to substitute the appropriate class for their cluster (e.g., replace efs-sc with the cluster-specific storage class); ensure the note references the storageClassName field and gives a one-line example instruction to choose the matching class per cluster.go.mod (1)
155-160:⚠️ Potential issue | 🟠 MajorUpgrade the OTel modules off the vulnerable
v1.41.0line.
go.opentelemetry.io/otel/sdk v1.41.0is still the version flagged by OSV for GHSA-hfvc-g4fc-pqhx. Since this PR is introducing tracing, I’d bump the OTel modules together to the first patched release before merging.What is the first patched version of go.opentelemetry.io/otel/sdk for GHSA-hfvc-g4fc-pqhx, and which go.opentelemetry.io/otel modules should be upgraded together from v1.41.0?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` around lines 155 - 160, Upgrade the vulnerable OTel modules from v1.41.0 to the first patched release (use v1.41.1) by updating all related import lines together: go.opentelemetry.io/otel/sdk, go.opentelemetry.io/otel, go.opentelemetry.io/otel/exporters/otlp/otlptrace, go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc, go.opentelemetry.io/otel/metric, and go.opentelemetry.io/otel/trace in go.mod so they are all bumped to the patched version (v1.41.1) and then run go mod tidy to lock the updated versions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/build-api/main.go`:
- Around line 69-70: The code now calls loadFromOperatorConfig(*namespace) which
uses the command-line namespace flag; change the call to pass the runtime
WATCH_NAMESPACE value instead (e.g., watchNamespace or
os.Getenv("WATCH_NAMESPACE")) so OperatorConfig lookup always uses the
operator's runtime namespace, and keep the existing namespace flag only as a
local/dev override (do not let it silently override the runtime lookup). Update
any related reads (e.g., readTracingConfig) or callers to consistently use
WATCH_NAMESPACE as the canonical source for OperatorConfig and remove fallbacks
to POD_NAMESPACE or serviceaccount namespace file when resolving OperatorConfig.
In `@cmd/main.go`:
- Around line 208-219: readTracingConfig currently collapses a disabled tracing
spec into an empty endpoint which causes telemetry.InitTracing to pick up OTEL
env vars; change readTracingConfig to return an explicit enabled bool (e.g.,
tracingEnabled) along with tracingEndpoint/tracingSamplingRatio/tracingInsecure,
then in cmd/main.go check that tracingEnabled is true before calling
telemetry.InitTracing and set shutdownTracing to a no-op when disabled; update/
add a unit/integration test that sets spec.tracing.enabled=false and
OTEL_EXPORTER_OTLP_ENDPOINT env var and asserts that InitTracing is not
invoked/that tracing remains disabled to prevent regression.
In `@docs/observability-setup.md`:
- Around line 319-321: The LogQL example uses the placeholder "<uuid>" which
conflicts with the document's defined trace-id format; update the LogQL sample
that filters on kubernetes_labels_automotive_sdv_cloud_redhat_com_trace_id to
use a consistent placeholder such as "<trace-id>" (and replace any other
"<uuid>" occurrences in this file) so the example matches the trace ID format
described elsewhere in the document.
- Around line 739-742: Add a deletion step for the cluster observability
subscription: remove the `cluster-observability-operator` subscription during
teardown by running `oc delete subscription cluster-observability-operator -n
<the-namespace-used-in-install>` (use the exact namespace where
`cluster-observability-operator` was installed in the install block), so the
teardown mirrors the install and cleans up that operator as well.
In `@internal/controller/imagebuild/controller.go`:
- Around line 208-215: Add failing unit tests that assert the root OpenTelemetry
span records errors for the early-return paths in
ImageBuildReconciler.Reconcile: specifically cover the branches where the
function returns before the end (the checks that currently exit before span.End
is called) so the tests fail with the current defer span.End() behavior;
implement tests that create a fake tracer/exporter or use a test span recorder
to verify span.Status/Error is set for those early returns, targeting the
Reconcile method on ImageBuildReconciler and the ibTracer/span usage; only after
these tests fail should you refactor to use
controllerutils.EndSpanWithError(span, &err) (or similar) to record errors and
make the tests pass.
---
Duplicate comments:
In `@docs/observability-setup.md`:
- Line 234: The docs currently hard-code storageClassName: efs-sc; update the
"storageClassName" entry to mention that "efs-sc" is environment-specific, add a
short note instructing readers to run `oc get storageclass` to list available
classes and to substitute the appropriate class for their cluster (e.g., replace
efs-sc with the cluster-specific storage class); ensure the note references the
storageClassName field and gives a one-line example instruction to choose the
matching class per cluster.
In `@go.mod`:
- Around line 155-160: Upgrade the vulnerable OTel modules from v1.41.0 to the
first patched release (use v1.41.1) by updating all related import lines
together: go.opentelemetry.io/otel/sdk, go.opentelemetry.io/otel,
go.opentelemetry.io/otel/exporters/otlp/otlptrace,
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc,
go.opentelemetry.io/otel/metric, and go.opentelemetry.io/otel/trace in go.mod so
they are all bumped to the patched version (v1.41.1) and then run go mod tidy to
lock the updated versions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9894152a-41f4-40b1-acb0-53bf8b2b5d17
⛔ Files ignored due to path filters (197)
config/crd/bases/automotive.sdv.cloud.redhat.com_operatorconfigs.yamlis excluded by!config/crd/bases/**go.sumis excluded by!**/*.sum,!**go.sumvendor/cel.dev/expr/BUILD.bazelis excluded by!vendor/**vendor/cel.dev/expr/MODULE.bazelis excluded by!vendor/**vendor/cel.dev/expr/checked.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/cel.dev/expr/eval.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/cel.dev/expr/explain.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/cel.dev/expr/syntax.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/cel.dev/expr/value.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/github.com/go-jose/go-jose/v4/README.mdis excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/asymmetric.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/cipher/key_wrap.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/crypter.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/shared.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/signing.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/symmetric.gois excluded by!vendor/**vendor/github.com/go-jose/go-jose/v4/symmetric_legacy.gois excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/BUILD.bazelis excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/context.gois excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/handler.gois excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/marshal_jsonpb.gois excluded by!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/mux.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/LICENSEis excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/exporter.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform/attribute.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform/instrumentation.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform/resource.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform/span.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/LICENSEis excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/client.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/counter/counter.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/gen.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/observ/doc.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/observ/instrumentation.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/observ/target.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/partialsuccess.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/retry/retry.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/version.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/x/README.mdis excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/x/observ.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/x/x.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/version.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/LICENSEis excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/internal/x/features.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/internal/x/x.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/builtin.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/container.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/env.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_bsd.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_linux.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_readfile.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_unsupported.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_windows.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os_release_unix.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os_unix.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os_unsupported.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/process.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/resource.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/batch_span_processor.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/doc.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/id_generator.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/env/env.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/batch_span_processor.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/doc.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/simple_span_processor.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/tracer.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/provider.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/sampling.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/simple_span_processor.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/snapshot.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/span.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/span_limits.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/tracer.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/version.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/sdk/version.gois excluded by!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.39.0/otelconv/metric.gois excluded by!vendor/**vendor/go.opentelemetry.io/proto/otlp/common/v1/common.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/go.opentelemetry.io/proto/otlp/resource/v1/resource.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/go.opentelemetry.io/proto/otlp/trace/v1/trace.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/golang.org/x/crypto/scrypt/scrypt.gois excluded by!vendor/**vendor/golang.org/x/net/html/node.gois excluded by!vendor/**vendor/golang.org/x/net/html/nodetype_string.gois excluded by!vendor/**vendor/golang.org/x/net/http2/client_priority_go126.gois excluded by!vendor/**vendor/golang.org/x/net/http2/client_priority_go127.gois excluded by!vendor/**vendor/golang.org/x/net/http2/frame.gois excluded by!vendor/**vendor/golang.org/x/net/http2/http2.gois excluded by!vendor/**vendor/golang.org/x/net/http2/server.gois excluded by!vendor/**vendor/golang.org/x/net/http2/transport.gois excluded by!vendor/**vendor/golang.org/x/net/http2/writesched_priority_rfc7540.gois excluded by!vendor/**vendor/golang.org/x/net/internal/httpsfv/httpsfv.gois excluded by!vendor/**vendor/golang.org/x/oauth2/deviceauth.gois excluded by!vendor/**vendor/golang.org/x/oauth2/oauth2.gois excluded by!vendor/**vendor/golang.org/x/oauth2/pkce.gois excluded by!vendor/**vendor/golang.org/x/oauth2/token.gois excluded by!vendor/**vendor/golang.org/x/oauth2/transport.gois excluded by!vendor/**vendor/golang.org/x/sys/cpu/cpu_arm64.gois excluded by!vendor/**vendor/golang.org/x/sys/cpu/cpu_other_arm64.gois excluded by!vendor/**vendor/golang.org/x/sys/cpu/cpu_windows_arm64.gois excluded by!vendor/**vendor/golang.org/x/sys/unix/ioctl_signed.gois excluded by!vendor/**vendor/golang.org/x/sys/unix/ioctl_unsigned.gois excluded by!vendor/**vendor/golang.org/x/sys/unix/syscall_solaris.gois excluded by!vendor/**vendor/golang.org/x/sys/unix/syscall_unix.gois excluded by!vendor/**vendor/golang.org/x/sys/windows/syscall_windows.gois excluded by!vendor/**vendor/golang.org/x/sys/windows/types_windows.gois excluded by!vendor/**vendor/golang.org/x/sys/windows/zsyscall_windows.gois excluded by!vendor/**vendor/golang.org/x/text/message/catalog/catalog.gois excluded by!vendor/**vendor/golang.org/x/text/message/catalog/dict.gois excluded by!vendor/**vendor/golang.org/x/text/message/catalog/go19.gois excluded by!vendor/**vendor/golang.org/x/text/message/catalog/gopre19.gois excluded by!vendor/**vendor/golang.org/x/text/secure/bidirule/bidirule.gois excluded by!vendor/**vendor/golang.org/x/text/secure/bidirule/bidirule10.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/secure/bidirule/bidirule9.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables10.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables11.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables12.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables13.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables15.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables17.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/bidi/tables9.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/forminfo.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables10.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables11.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables12.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables15.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables17.0.0.gois excluded by!vendor/**vendor/golang.org/x/text/unicode/norm/tables9.0.0.gois excluded by!vendor/**vendor/google.golang.org/grpc/CONTRIBUTING.mdis excluded by!vendor/**vendor/google.golang.org/grpc/balancer/balancer.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/pickfirst/internal/internal.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/pickfirst/pickfirst.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/pickfirst/pickfirstleaf/pickfirstleaf.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/roundrobin/roundrobin.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer/subconn.gois excluded by!vendor/**vendor/google.golang.org/grpc/balancer_wrapper.gois excluded by!vendor/**vendor/google.golang.org/grpc/binarylog/grpc_binarylog_v1/binarylog.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/google.golang.org/grpc/clientconn.gois excluded by!vendor/**vendor/google.golang.org/grpc/credentials/credentials.gois excluded by!vendor/**vendor/google.golang.org/grpc/credentials/tls.gois excluded by!vendor/**vendor/google.golang.org/grpc/encoding/encoding.gois excluded by!vendor/**vendor/google.golang.org/grpc/encoding/gzip/gzip.gois excluded by!vendor/**vendor/google.golang.org/grpc/encoding/internal/internal.gois excluded by!vendor/**vendor/google.golang.org/grpc/encoding/proto/proto.gois excluded by!vendor/**vendor/google.golang.org/grpc/experimental/stats/metricregistry.gois excluded by!vendor/**vendor/google.golang.org/grpc/experimental/stats/metrics.gois excluded by!vendor/**vendor/google.golang.org/grpc/health/grpc_health_v1/health.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/google.golang.org/grpc/health/grpc_health_v1/health_grpc.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/google.golang.org/grpc/interceptor.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/balancer/gracefulswitch/gracefulswitch.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/balancer/weight/weight.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/buffer/unbounded.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/channelz/trace.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/envconfig/envconfig.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/envconfig/xds.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/experimental.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/grpcsync/callback_serializer.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/idle/idle.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/internal.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/resolver/delegatingresolver/delegatingresolver.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/resolver/dns/dns_resolver.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/stats/metrics_recorder_list.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/stats/stats.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/client_stream.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/controlbuf.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/flowcontrol.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/handler_server.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/http2_client.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/http2_server.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/http_util.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/server_stream.gois excluded by!vendor/**vendor/google.golang.org/grpc/internal/transport/transport.gois excluded by!vendor/**vendor/google.golang.org/grpc/mem/buffer_pool.gois excluded by!vendor/**vendor/google.golang.org/grpc/mem/buffer_slice.gois excluded by!vendor/**vendor/google.golang.org/grpc/mem/buffers.gois excluded by!vendor/**vendor/google.golang.org/grpc/preloader.gois excluded by!vendor/**vendor/google.golang.org/grpc/resolver/resolver.gois excluded by!vendor/**vendor/google.golang.org/grpc/resolver_wrapper.gois excluded by!vendor/**vendor/google.golang.org/grpc/rpc_util.gois excluded by!vendor/**vendor/google.golang.org/grpc/server.gois excluded by!vendor/**vendor/google.golang.org/grpc/stream.gois excluded by!vendor/**vendor/google.golang.org/grpc/version.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/encoding/tag/tag.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/encoding/text/decode.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/filedesc/desc.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/filedesc/desc_lazy.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/genid/descriptor_gen.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/impl/codec_map.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/impl/decode.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/impl/validate.gois excluded by!vendor/**vendor/google.golang.org/protobuf/internal/version/version.gois excluded by!vendor/**vendor/google.golang.org/protobuf/proto/decode.gois excluded by!vendor/**vendor/google.golang.org/protobuf/reflect/protodesc/desc.gois excluded by!vendor/**vendor/google.golang.org/protobuf/reflect/protodesc/editions.gois excluded by!vendor/**vendor/google.golang.org/protobuf/types/descriptorpb/descriptor.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/google.golang.org/protobuf/types/known/timestamppb/timestamp.pb.gois excluded by!**/*.pb.go,!vendor/**vendor/modules.txtis excluded by!vendor/**
📒 Files selected for processing (21)
api/v1alpha1/labels.goapi/v1alpha1/operatorconfig_types.goapi/v1alpha1/zz_generated.deepcopy.gocmd/build-api/main.gocmd/main.godocs/observability-setup.mdgo.modinternal/buildapi/openapi.yamlinternal/buildapi/server.gointernal/buildapi/types.gointernal/common/tasks/scripts/common.shinternal/common/tasks/tasks.gointernal/common/tasks/traceid_test.gointernal/common/telemetry/tracing.gointernal/common/telemetry/tracing_test.gointernal/controller/controllerutils/labels.gointernal/controller/controllerutils/labels_test.gointernal/controller/controllerutils/tracing.gointernal/controller/imagebuild/controller.gointernal/controller/operatorconfig/controller.gointernal/controller/workspace/controller.go
✅ Files skipped from review due to trivial changes (2)
- internal/common/tasks/scripts/common.sh
- api/v1alpha1/labels.go
🚧 Files skipped from review as they are similar to previous changes (7)
- internal/buildapi/types.go
- internal/controller/controllerutils/labels.go
- internal/controller/controllerutils/tracing.go
- internal/common/tasks/traceid_test.go
- internal/common/telemetry/tracing.go
- internal/controller/controllerutils/labels_test.go
- internal/controller/operatorconfig/controller.go
bkhizgiy
approved these changes
Apr 27, 2026
bennyz
force-pushed
the
worktree-monitoring-config
branch
from
April 27, 2026 10:46
af48c3d to
82af181
Compare
✅ Actions performedReview triggered.
|
Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com> Assisted-by: claude-opus-4.6
bennyz
force-pushed
the
worktree-monitoring-config
branch
from
April 27, 2026 11:28
82af181 to
1cfd36b
Compare
10 tasks
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
2 participants
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.
Summary
Related Issues
Type of Change
Testing
make test)make lint)make manifests generate)Summary by CodeRabbit
New Features
Documentation
Bug Fixes / Improvements
Tests
Chores