feat: support OTLP tracing protocol selection by houyuwushang · Pull Request #1050 · trickstercache/trickster

houyuwushang · 2026-07-04T08:27:00Z

Description

Fixes #381.

This adds explicit OTLP transport selection for tracing configs with protocol: http|grpc:

keeps existing OTLP configs backward-compatible by defaulting omitted OTLP protocol to http
preserves the existing OTLP/HTTP endpoint handling
adds OTLP/gRPC exporter support, including timeout, headers, and compression options
rejects path-only endpoints for gRPC because that form is HTTP-specific
documents HTTP and gRPC Jaeger examples and updates runnable demo configs to set protocol: http explicitly

I think this completes the remaining OTLP transport/config gap after the earlier #381 tracing slices landed.

Validation:

go test ./pkg/observability/tracing/options -count=1
go test ./pkg/observability/tracing/exporters/otlp -count=1
go test ./pkg/observability/tracing/... -count=1
go test ./pkg/observability/... -count=1
go test -race ./pkg/observability/tracing/exporters/otlp -run TestNewAppliesGRPCProtocol -count=1
go tool golangci-lint run --timeout 5m ./pkg/observability/tracing/...
go test -run '^$' ./...
go test ./pkg/config/validate -count=1
go run ./cmd/trickster -validate-config -config examples\docker-compose\docker-compose-data\trickster-config\trickster.yaml
go run ./cmd/trickster -validate-config -config docs\developer\environment\trickster-config\trickster.yaml
git diff --check

Note: I also tried go test ./pkg/config/... locally on Windows; it hits the existing TestConfig_defaulting golden/path separator difference (/tmp/trickster vs .\cache), so I did not list it as passing validation above.

Type of Change

- Bug fix
- New feature
- Optimization
- Test coverage
- Documentation
- Infrastructure

AI Disclosure

- This contribution DOES NOT include AI-generated changes
- This contribution DOES include AI-generated changes, and I have reviewed the relevant contributing guidelines.

houyuwushang · 2026-07-04T08:32:05Z

Update: the first integration CI run failed before executing integration tests because integration/ is a separate Go module and its go.sum was missing the new otlptracegrpc checksum. I added the corresponding integration/go.mod / integration/go.sum entries and force-pushed the amended commit.

Additional local validation for that fix:

cd integration; GOOS=linux GOARCH=amd64 CGO_ENABLED=0 go test -c -o $TMP/trickster-integration.test
go test ./pkg/observability/tracing/... -count=1
git diff --check

houyuwushang · 2026-07-04T15:00:25Z

Small follow-up after a final sweep: since this PR adds OTLP/gRPC support, I also exposed Jaeger's 4317 port in both compose examples and clarified the docs for host-vs-compose gRPC endpoints. That keeps the runnable demo environment aligned with the new protocol: grpc option.

Additional local validation:

go test ./pkg/observability/tracing/... -count=1
go run ./cmd/trickster -validate-config -config examples\docker-compose\docker-compose-data\trickster-config\trickster.yaml
go run ./cmd/trickster -validate-config -config docs\developer\environment\trickster-config\trickster.yaml
git diff --check

houyuwushang · 2026-07-05T06:12:52Z

Small final coverage follow-up: I added a config validation test proving an invalid tracing protocol is rejected through the top-level tracing validation path, not only at the options package level. No behavior change beyond the existing validation in this PR.

Additional local validation after the amend:

go test ./pkg/config/validate -count=1
go test ./pkg/observability/tracing/... -count=1
go tool golangci-lint run --timeout 5m ./pkg/config/validate ./pkg/observability/tracing/...
go test -run '^$' ./...
go run ./cmd/trickster -validate-config -config examples\docker-compose\docker-compose-data\trickster-config\trickster.yaml
go run ./cmd/trickster -validate-config -config docs\developer\environment\trickster-config\trickster.yaml
git diff --check

jranson

looks generally good, just a few minor nits around use of string literals versus consts, and some error cleanup.

jranson · 2026-07-06T18:10:14Z

+		return nil, err
+	}
+
+	tracerOpts := make([]sdktrace.TracerProviderOption, 0, 3)


You should be able bootstrap tp in a single line without creating/appending tracerOpts, like:

tp = sdktrace.NewTracerProvider( sdktrace.WithSampler(tracing.Sampler(o)), sdktrace.WithResource(resource.NewWithAttributes("", tags...)), sdktrace.WithBatcher(exporter), )

jranson · 2026-07-06T18:17:23Z

+	switch {
+	case o.Endpoint == "":
+	case strings.HasPrefix(o.Endpoint, "/"):
+		return nil, fmt.Errorf("%w: path-only endpoints require OTLP/HTTP protocol",


prefer a static package-level error here. a dynamically-created error is not necessary (the error message will always be the same) and unit tests can then use a simple pointer equality test on the error var for verification instead of errors.Is().

jranson · 2026-07-06T18:21:06Z

 		if v.Provider == "" {
 			v.Provider = DefaultTracerProvider
 		}
+		if v.Provider == "otlp" && v.Protocol == "" {


should create and use an OTLP constant here instead of the "otlp" string literal.

jranson · 2026-07-06T18:22:20Z

 	}
+
+	otlp := &Options{Provider: "otlp"}
+	ProcessTracingOptions(Lookup{"otlp": otlp})


should use an OTLP constant here instead of the "otlp" string literal.

jranson · 2026-07-06T18:27:55Z

+	opt.Protocol = options.OTLPProtocolGRPC
+	opt.Endpoint = "/v1/traces"
+	_, err = New(opt)
+	if !errors.Is(err, errs.ErrInvalidEndpointURL) {


here is where you can check for the requested package-level var instead of using errors.Is()

jranson · 2026-07-06T18:30:22Z

+	opt.Protocol = options.OTLPProtocolGRPC
+	opt.Endpoint = "http://" + lis.Addr().String()
+	opt.ServiceName = "trickster-grpc-test"
+	opt.Headers = map[string]string{"x-trickster-test": "grpc"}


use options.OTLPProtocolGRPC here instead of the string literal

jranson · 2026-07-06T18:31:06Z

+	select {
+	case md := <-metadataCh:
+		if got := md.Get("x-trickster-test"); len(got) != 1 || got[0] != "grpc" {
+			t.Errorf("expected gRPC metadata header %q, got %v", "grpc", got)


use options.OTLPProtocolGRPC in the conditional check and the test error

houyuwushang · 2026-07-07T00:20:43Z

Thanks, fixed the requested nits:

added and used options.ProviderOTLP instead of OTLP provider string literals in the options default path/tests
changed the gRPC path-only endpoint error to a package-level var and updated the test to compare that var directly
simplified OTLP tracer provider construction to pass options directly to sdktrace.NewTracerProvider(...)
replaced the remaining gRPC test header literal with options.OTLPProtocolGRPC

Validated with:

go test ./pkg/observability/tracing/options ./pkg/observability/tracing/exporters/otlp -count=1
go test ./pkg/observability/tracing/... ./pkg/config/validate -count=1
go test -run '^$' ./...
go tool golangci-lint run --timeout 5m ./pkg/observability/tracing/... ./pkg/config/validate
git diff --check

Signed-off-by: houyuwushang <liuluoqianqiu@outlook.com>

houyuwushang requested a review from a team as a code owner July 4, 2026 08:27

houyuwushang mentioned this pull request Jul 4, 2026

Make Tracing Configurability More Robust #381

Closed

houyuwushang force-pushed the tracing-otlp-protocol-381 branch from efd69e0 to 1392242 Compare July 4, 2026 08:31

houyuwushang force-pushed the tracing-otlp-protocol-381 branch from 1392242 to a7485e4 Compare July 4, 2026 15:00

houyuwushang force-pushed the tracing-otlp-protocol-381 branch from a7485e4 to 5ccb749 Compare July 5, 2026 06:12

jranson requested changes Jul 6, 2026

View reviewed changes

houyuwushang force-pushed the tracing-otlp-protocol-381 branch from 5ccb749 to 410b054 Compare July 7, 2026 00:20

feat: support OTLP tracing protocol selection

93e452d

Signed-off-by: houyuwushang <liuluoqianqiu@outlook.com>

houyuwushang force-pushed the tracing-otlp-protocol-381 branch from 410b054 to 93e452d Compare July 7, 2026 00:21

jranson approved these changes Jul 7, 2026

View reviewed changes

jranson merged commit 0a4173d into trickstercache:main Jul 7, 2026
8 checks passed

Uh oh!

Conversation

houyuwushang commented Jul 4, 2026

Description

Type of Change

AI Disclosure

Uh oh!

houyuwushang commented Jul 4, 2026

Uh oh!

houyuwushang commented Jul 4, 2026

Uh oh!

houyuwushang commented Jul 5, 2026

Uh oh!

jranson left a comment

Choose a reason for hiding this comment

Uh oh!

jranson Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

jranson Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

jranson Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jranson Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jranson Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

jranson Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

jranson Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

houyuwushang commented Jul 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jranson Jul 6, 2026 •

edited

Loading

jranson Jul 6, 2026 •

edited

Loading

jranson Jul 6, 2026 •

edited

Loading