Skip to content

feat(tracing): add tracing support and configuration options for BBR #2518

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
k8s-ci-robot merged 2 commits into from
Mar 11, 2026
Merged

feat(tracing): add tracing support and configuration options for BBR #2518

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
141157c
feat(tracing): add tracing support and configuration options for BBR
AtharvaPakade Mar 8, 2026
bd60ece
refactor(tracing): make InitTracing generic and unify instrumentation…
AtharvaPakade Mar 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions cmd/bbr/runner/runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import (
runserver "sigs.k8s.io/gateway-api-inference-extension/pkg/bbr/server"
logutil "sigs.k8s.io/gateway-api-inference-extension/pkg/common/observability/logging"
"sigs.k8s.io/gateway-api-inference-extension/pkg/common/observability/profiling"
"sigs.k8s.io/gateway-api-inference-extension/pkg/common/observability/tracing"
"sigs.k8s.io/gateway-api-inference-extension/version"
)

Expand Down Expand Up @@ -102,6 +103,15 @@ func (r *Runner) Run(ctx context.Context) error {
pflag.VisitAll(func(f *pflag.Flag) {
flags[f.Name] = f.Value
})

if opts.Tracing {
err := tracing.InitTracing(ctx, setupLog, "gateway-api-inference-extension/bbr")
if err != nil {
setupLog.Error(err, "failed to initialize tracing")
return err
}
}

Comment on lines +107 to +114

@nirrozenbaum nirrozenbaum Mar 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I understand that the intention is to keep separate traces between bbr/epp.
two points about this:

  1. we should generalize tracing helper functions under common/observability/tracing in a consumable way, to allow both epp/bbr consume it and make their own traces independently.
  2. at the same time, we should also allow (in epp, not here) to continue the same trace from bbr. technically speaking, if we want to see a full trace of the request (including bbr + epp) that is very useful.
    having said that, this point is not required in current PR (can be done in follow ups), but just something to keep in mind.

@AtharvaPakade AtharvaPakade Mar 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

  1. I do plan to do that, but just avoided it for now in this PR as epp changes will also required. In futhure PR i will make this uniform, just to give a hint i was planning some pacakge level tracing config that will be initialized by common helper.
  2. So i do have this in mind, i have not drafted a proposal yet but there are few things i have indetified which can add a lot of values to tracing in general.

setupLog.Info("Flags processed", "flags", flags)

logutil.InitLogging(&opts.ZapOptions)
Expand Down
2 changes: 1 addition & 1 deletion cmd/epp/runner/runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ func (r *Runner) Run(ctx context.Context) error {
logutil.InitLogging(&opts.ZapOptions)

if opts.Tracing {
err := tracing.InitTracing(ctx, setupLog)
err := tracing.InitTracing(ctx, setupLog, "gateway-api-inference-extension/epp")
if err != nil {
return fmt.Errorf("failed to init tracing %w", err)
}
Expand Down
31 changes: 30 additions & 1 deletion config/charts/body-based-routing/templates/bbr.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,42 @@ spec:
- {{ printf "%s:%s" .type .name | quote }}
{{- end }}
{{- end }}
{{- if not .Values.bbr.multiNamespace }}
{{- if .Values.bbr.tracing.enabled}}
- --tracing=true
{{- else}}
- --tracing=false
{{- end}}
Comment on lines +36 to +40

@nirrozenbaum nirrozenbaum Mar 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is it possible to align with other flags using key/value in separate cmd-line flags? does it work?

Suggested change
{{- if .Values.bbr.tracing.enabled}}
- --tracing=true
{{- else}}
- --tracing=false
{{- end}}
{{- if .Values.bbr.tracing.enabled}}
- --tracing
- "true"
{{- else}}
- --tracing
- "false"
{{- end}}

@AtharvaPakade AtharvaPakade Mar 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This will not work because it resolves to --tracing <value>. Since tracing is a boolean flag, the Go flag parser treats the presence of --tracing as true, and the following value is interpreted as a positional argument rather than the flag’s value.

env:
{{- if not .Values.bbr.multiNamespace }}
- name: NAMESPACE
valueFrom:
fieldRef:
fieldPath: metadata.namespace
{{- end }}
{{- if .Values.bbr.tracing.enabled}}
Comment on lines +42 to +48

@nirrozenbaum nirrozenbaum Mar 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

out of curiosity, is leaving env with no values under it valid?
e.g., if multiNamespace is false and tracing.enabled is false.

@AtharvaPakade AtharvaPakade Mar 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is valid and works, but it would be better to add another if to check if one of the two condition are true.

@nirrozenbaum nirrozenbaum Mar 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you can leave it as is for now since empty env is working.
there is a planned change for the multiNamespace anyway so we'll need to do some changes here.

- name: OTEL_SERVICE_NAME
value: {{ .Values.bbr.tracing.otelServiceName }}
- name: OTEL_EXPORTER_OTLP_ENDPOINT
value: {{ .Values.bbr.tracing.otelExporterEndpoint }}
- name: OTEL_RESOURCE_ATTRIBUTES_NODE_NAME
valueFrom:
fieldRef:
apiVersion: v1
fieldPath: spec.nodeName
- name: OTEL_RESOURCE_ATTRIBUTES_POD_NAME
valueFrom:
fieldRef:
apiVersion: v1
fieldPath: metadata.name
- name: OTEL_RESOURCE_ATTRIBUTES
value: 'k8s.namespace.name=$(NAMESPACE),k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME),k8s.pod.name=$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME)'
- name: OTEL_TRACES_SAMPLER
value: {{ .Values.bbr.tracing.sampling.sampler | quote }}
- name: OTEL_TRACES_SAMPLER_ARG
value: {{ .Values.bbr.tracing.sampling.samplerArg | quote }}
- name: OTEL_TRACES_EXPORTER
value: otlp
{{- end}}
ports:
- containerPort: {{ .Values.bbr.port }}
# health check
Expand Down
8 changes: 8 additions & 0 deletions config/charts/body-based-routing/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ bbr:
# Log verbosity
v: 3

tracing:
enabled: false
otelServiceName: "gateway-api-inference-extension/bbr"
otelExporterEndpoint: "http://localhost:4317"
sampling:
sampler: "parentbased_traceidratio"
samplerArg: "0.1"

provider:
name: none

Expand Down
2 changes: 1 addition & 1 deletion config/charts/epplib/templates/_deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ spec:
{{- include "gateway-api-inference-extension.latencyPredictor.env" . | nindent 12 }}
{{- if .Values.inferenceExtension.tracing.enabled }}
- name: OTEL_SERVICE_NAME
value: "gateway-api-inference-extension"
value: "gateway-api-inference-extension/epp"
- name: OTEL_EXPORTER_OTLP_ENDPOINT
value: {{ .Values.inferenceExtension.tracing.otelExporterEndpoint | quote }}
- name: OTEL_TRACES_EXPORTER
Expand Down
16 changes: 16 additions & 0 deletions pkg/bbr/handlers/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ import (
"time"

extProcPb "github.com/envoyproxy/go-control-plane/envoy/service/ext_proc/v3"
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/trace"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"sigs.k8s.io/controller-runtime/pkg/log"
Expand All @@ -31,6 +34,7 @@ import (
reqenvoy "sigs.k8s.io/gateway-api-inference-extension/pkg/common/envoy/request"
logutil "sigs.k8s.io/gateway-api-inference-extension/pkg/common/observability/logging"
reqcommon "sigs.k8s.io/gateway-api-inference-extension/pkg/common/request"
"sigs.k8s.io/gateway-api-inference-extension/version"
)

type Datastore interface {
Expand Down Expand Up @@ -77,6 +81,18 @@ type Response struct {

func (s *Server) Process(srv extProcPb.ExternalProcessor_ProcessServer) error {
ctx := srv.Context()

// Start tracing span for the request
tracer := otel.Tracer(
"gateway-api-inference-extension/bbr/extproc",
trace.WithInstrumentationVersion(version.BuildRef),
trace.WithInstrumentationAttributes(
attribute.String("commit-sha", version.CommitSHA),
),
)
ctx, span := tracer.Start(ctx, "gateway.request", trace.WithSpanKind(trace.SpanKindServer))
defer span.End()

logger := log.FromContext(ctx)
loggerVerbose := logger.V(logutil.VERBOSE)
loggerVerbose.Info("Processing")
Expand Down
3 changes: 3 additions & 0 deletions pkg/bbr/server/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ type Options struct {
// Diagnostics.
//
logging.LoggingOptions // Logging configuration.
Tracing bool // Enable emitting traces
MetricsPort int // The metrics port exposed by BBR.
GRPCHealthPort int // The port for gRPC liveness and readiness probes.
EnablePprof bool // Enables pprof handlers.
Expand All @@ -61,6 +62,7 @@ func NewOptions() *Options {
GRPCPort: DefaultGrpcPort,
GRPCHealthPort: DefaultGrpcHealthPort,
LoggingOptions: *logging.NewOptions(),
Tracing: true,
MetricsPort: 9090,
EnablePprof: true,
SecureServing: true,
Expand All @@ -82,6 +84,7 @@ func (opts *Options) AddFlags(fs *pflag.FlagSet) {
"The port used for gRPC liveness and readiness probes.")
fs.IntVar(&opts.MetricsPort, "metrics-port", opts.MetricsPort,
"The metrics port exposed by BBR.")
fs.BoolVar(&opts.Tracing, "tracing", opts.Tracing, "Enables emitting traces.")
fs.BoolVar(&opts.MetricsEndpointAuth, "metrics-endpoint-auth", opts.MetricsEndpointAuth,
"Enables authentication and authorization of the metrics endpoint.")
fs.BoolVar(&opts.Streaming, "streaming", opts.Streaming,
Expand Down
4 changes: 2 additions & 2 deletions pkg/common/observability/tracing/telemetry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,13 @@ func (h *errorHandler) Handle(err error) {
h.logger.V(logging.DEFAULT).Error(err, "trace error occurred")
}

func InitTracing(ctx context.Context, logger logr.Logger) error {
func InitTracing(ctx context.Context, logger logr.Logger, defaultServiceName string) error {
logger = logger.WithName("trace")
loggerWrap := &errorHandler{logger: logger}

_, ok := os.LookupEnv("OTEL_SERVICE_NAME")
if !ok {
os.Setenv("OTEL_SERVICE_NAME", "gateway-api-inference-extension")
os.Setenv("OTEL_SERVICE_NAME", defaultServiceName)
}

_, ok = os.LookupEnv("OTEL_EXPORTER_OTLP_ENDPOINT")
Expand Down
10 changes: 9 additions & 1 deletion pkg/epp/handlers/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/go-logr/logr"
"github.com/google/uuid"
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/trace"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
Expand All @@ -43,6 +44,7 @@ import (
fwkrh "sigs.k8s.io/gateway-api-inference-extension/pkg/epp/framework/interface/requesthandling"
schedulingtypes "sigs.k8s.io/gateway-api-inference-extension/pkg/epp/framework/interface/scheduling"
"sigs.k8s.io/gateway-api-inference-extension/pkg/epp/metrics"
"sigs.k8s.io/gateway-api-inference-extension/version"
)

func NewStreamingServer(datastore Datastore, director Director, parser fwkrh.Parser) *StreamingServer {
Expand Down Expand Up @@ -137,7 +139,13 @@ func (s *StreamingServer) Process(srv extProcPb.ExternalProcessor_ProcessServer)
ctx := srv.Context()

// Start tracing span for the request
tracer := otel.Tracer("gateway-api-inference-extension")
tracer := otel.Tracer(
"gateway-api-inference-extension/epp/extproc",
trace.WithInstrumentationVersion(version.BuildRef),
trace.WithInstrumentationAttributes(
attribute.String("commit-sha", version.CommitSHA),
),
)
ctx, span := tracer.Start(ctx, "gateway.request", trace.WithSpanKind(trace.SpanKindServer))
defer span.End()

Expand Down