feat: [#726][Trace] Add OpenTelemetry Tracing Support for Distributed Tracing #1283

kkumar-gcc · 2025-12-04T13:56:04Z

📑 Description

RelatedTo goravel/goravel#726

Summary

Introduces a new Telemetry module providing OpenTelemetry-based distributed tracing for Goravel applications.

Features:

Multiple exporters: OTLP, Zipkin, Console
Configurable samplers: always_on, always_off, traceidratio
Context propagation: W3C TraceContext, Baggage, B3

Note

Currently It doesn't support custom exporter, I will add this feature in next couple of PR

Usage

Configuration (`.env`)

OTEL_SERVICE_NAME=my-service
OTEL_TRACES_EXPORTER=otlp
OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4318

Creating Traces

tracer := facades.Telemetry().Tracer("my-service")

ctx, span := tracer.Start(ctx, "operation-name")
defer span.End()

span.SetAttributes(attribute.String("user.id", "123"))

Context Propagation

// Inject trace context into outgoing HTTP requests
facades.Telemetry().Propagator().Inject(ctx, telemetry.PropagationHeaderCarrier(req.Header))

// Extract trace context from incoming HTTP requests
ctx = facades.Telemetry().Propagator().Extract(ctx, telemetry.PropagationHeaderCarrier(req.Header))

Shutdown

defer facades.Telemetry().Shutdown(context.Background())

Configuration Reference

Option	Env Variable	Default	Description
Exporter	`OTEL_TRACES_EXPORTER`	`otlp`	`otlp`, `zipkin`, `console`, `none`
Sampler	`OTEL_TRACES_SAMPLER_TYPE`	`always_on`	`always_on`, `always_off`, `traceidratio`
Propagators	`OTEL_PROPAGATORS`	`tracecontext,baggage`	`tracecontext`, `baggage`, `b3`, `b3multi`

✅ Checks

Added test cases for my code

codecov · 2025-12-04T13:58:17Z

Codecov Report

❌ Patch coverage is 63.81579% with 110 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.49%. Comparing base (d029a42) to head (d4fbba4).
⚠️ Report is 6 commits behind head on master.

Files with missing lines	Patch %	Lines
telemetry/setup/stubs.go	0.00%	54 Missing ⚠️
telemetry/setup/setup.go	0.00%	20 Missing ⚠️
telemetry/service_provider.go	0.00%	14 Missing ⚠️
foundation/container.go	0.00%	6 Missing ⚠️
support/config/key.go	0.00%	4 Missing ⚠️
telemetry/alias.go	0.00%	4 Missing ⚠️
telemetry/exporter.go	93.93%	2 Missing and 2 partials ⚠️
telemetry/application.go	97.56%	1 Missing and 1 partial ⚠️
telemetry/resource.go	90.47%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1283      +/-   ##
==========================================
- Coverage   68.56%   68.49%   -0.07%     
==========================================
  Files         264      274      +10     
  Lines       15566    15945     +379     
==========================================
+ Hits        10673    10922     +249     
- Misses       4430     4553     +123     
- Partials      463      470       +7

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

This PR claims to add OpenTelemetry tracing support for distributed tracing (issue #726), but the implementation is incomplete. The PR only creates a placeholder file with no actual functionality, no OpenTelemetry dependencies, and no tests.

Key observations:

Only a single file is added: contracts/telemetry/telemetry.go containing only a package declaration and a placeholder comment
No OpenTelemetry dependencies are added to go.mod
No actual tracing interfaces, types, or implementation code exists
No test coverage is provided

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

contracts/telemetry/telemetry.go

Copilot

Pull request overview

Copilot reviewed 26 out of 27 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-07T10:50:18Z

telemetry/setup/stubs.go

+		// Exporters Configuration
+		//
+		// Configure exporters for sending telemetry data.
+		// Supported drivers: "otlp", "zipkin", "console"


The comment says "Supported drivers" but should say "Supported exporters" for consistency with the rest of the configuration comments and the actual setting name ("exporter").

Suggested change

// Supported drivers: "otlp", "zipkin", "console"

// Supported exporters: "otlp", "zipkin", "console"

github-actions

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.

Benchmark suite	Current: `212a54b`	Previous: `6ef4277`	Ratio
`Benchmark_DecryptString`	`5648` ns/op 2032 B/op 16 allocs/op	`2017` ns/op 2032 B/op 16 allocs/op	`2.80`
`Benchmark_DecryptString - ns/op`	`5648` ns/op	`2017` ns/op	`2.80`
`BenchmarkFile_ReadWrite`	`197815` ns/op 6258 B/op 99 allocs/op	`128378` ns/op 6257 B/op 99 allocs/op	`1.54`
`BenchmarkFile_ReadWrite - ns/op`	`197815` ns/op	`128378` ns/op	`1.54`

This comment was automatically generated by workflow using github-action-benchmark.

Copilot

Pull request overview

Copilot reviewed 27 out of 28 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-07T13:42:53Z

telemetry/resource.go

+
+	"go.opentelemetry.io/otel/attribute"
+	"go.opentelemetry.io/otel/sdk/resource"
+	semconv "go.opentelemetry.io/otel/semconv/v1.37.0"


The import semconv "go.opentelemetry.io/otel/semconv/v1.37.0" is used but this dependency is not declared in go.mod or go.sum. This will cause compilation failures. The semconv package needs to be added as a direct dependency in go.mod.

Because this is not direct dependency but a subpart of otel#v1.138.0 package

support/config/key.go

kkumar-gcc · 2025-12-07T13:50:17Z

support/config/key.go

+//
+//	cfg.GetString(ConfigServiceName.String())
+//	cfg.GetString(ConfigExporter.With("otlp"))
+type Key string


Previously, configuration keys were declared as plain strings on-demand throughout the framework, making them inconsistent and error-prone. This package (support/config) provides a Key type that each package can use to define their own configuration keys in a consistent manner. (open to move the type in different package)

hwbrzzl

Awesome 👍 Two middleware (http/grpc) should be needed, correct? Normally, users don't need to use facades.Telemetry() directly.

hwbrzzl · 2025-12-08T01:56:45Z

telemetry/setup/setup.go

+		Install(
+			modify.AddProviderApply(modulePath, telemetryServiceProvider),
+			modify.File(telemetryConfigPath).Overwrite(stubs.Config(packages.GetModuleNameFromArgs(os.Args))),
+			modify.WhenFacade(facades.Telemetry, modify.File(telemetryFacadePath).Overwrite(stubs.TelemetryFacade())),


There is only one facade, so modify.WhenFacade and modify.WhenNoFacades are unnecessary here. I'm going to remove others later.

hwbrzzl · 2025-12-08T02:01:45Z

telemetry/setup/stubs.go

+			"name":        config.Env("OTEL_SERVICE_NAME", "goravel"),
+			"version":     config.Env("OTEL_SERVICE_VERSION", ""),
+			"environment": config.Env("OTEL_SERVICE_ENVIRONMENT", ""),


How about using APP_NAME, APP_ENV, and APP_VERSION?

hwbrzzl · 2025-12-08T02:08:15Z

telemetry/setup/stubs.go

+		// Propagators
+		//
+		// Propagators define how trace context is passed between services.
+		// Supported: "tracecontext", "baggage", "b3", "b3multi", "none"


When will the none be used? Can it be removed and use empty instead?

hwbrzzl · 2025-12-08T02:10:55Z

telemetry/setup/stubs.go

+		// Configure distributed tracing for your application.
+		"traces": map[string]any{
+			// The exporter determines where traces are sent.
+			// Supported: "otlp", "zipkin", "console", "none"


hwbrzzl · 2025-12-08T02:19:13Z

telemetry/setup/stubs.go

+				"args": map[string]any{
+					// Sampling ratio for "traceidratio" (0.0 to 1.0)
+					"ratio": config.Env("OTEL_TRACES_SAMPLER_RATIO", 0.05),
+				},


How about args.ratio -> ratio? Add a parent level args seems to be unnecessary.

hwbrzzl · 2025-12-08T02:45:46Z

telemetry/propagator.go

+	if len(propagators) == 0 {
+		return defaultCompositePropagator
+	}


Suggested change

if len(propagators) == 0 {

return defaultCompositePropagator

}

hwbrzzl · 2025-12-08T02:46:40Z

telemetry/propagator.go

+	if nameStr == "" {
+		return defaultCompositePropagator
+	}
+
+	if nameStr == propagatorNone {
+		return nonePropagator
+	}


propagators should be declared.

Suggested change

if nameStr == "" {

return defaultCompositePropagator

}

if nameStr == propagatorNone {

return nonePropagator

}

hwbrzzl · 2025-12-08T02:59:15Z

telemetry/application.go

+	exporterName := r.config.GetString(configTracesExporter.String())
+	if exporterName == "" || exporterName == "none" {
+		return nil
+	}


This judgment can be moved to createExporter.

hwbrzzl · 2025-12-08T03:00:55Z

telemetry/application.go

+	if r.propagator == nil {
+		return defaultCompositePropagator
+	}


r.propagator needs to respect newCompositeTextMapPropagator's result, I think. r.propagator should be not reset here.

Suggested change

if r.propagator == nil {

return defaultCompositePropagator

}

hwbrzzl · 2025-12-08T03:10:53Z

telemetry/application.go

+	propagator     propagation.TextMapPropagator
+}
+
+func NewApplication(cfg config.Config) (*Application, error) {


When writing test cases, we have to mock every configuration. Sometimes it's very complicated. Ideally, every module should have its own config interface to fetch configuration, and have a Config struct to be initiated in NewApplication, then the config can be mocked. We can optimized it later if you agree.

init telemetry contracts package

0acd121

Copilot AI review requested due to automatic review settings December 4, 2025 13:56

kkumar-gcc requested a review from a team as a code owner December 4, 2025 13:56

kkumar-gcc marked this pull request as draft December 4, 2025 13:56

Copilot started reviewing on behalf of kkumar-gcc December 4, 2025 13:56 View session

kkumar-gcc changed the title ~~feat: [#726] Add OpenTelemetry Tracing Support for Distributed Tracing~~ feat: [#726] Add OpenTelemetry Tracing Support for Distributed Tracing [Trace] Dec 4, 2025

kkumar-gcc changed the title ~~feat: [#726] Add OpenTelemetry Tracing Support for Distributed Tracing [Trace]~~ feat: [#726][Trace] Add OpenTelemetry Tracing Support for Distributed Tracing Dec 4, 2025

Copilot finished reviewing on behalf of kkumar-gcc December 4, 2025 13:58

Copilot AI reviewed Dec 4, 2025

View reviewed changes

contracts/telemetry/telemetry.go Outdated Show resolved Hide resolved

contracts/telemetry/telemetry.go Outdated Show resolved Hide resolved

contracts/telemetry/telemetry.go Outdated Show resolved Hide resolved

contracts/telemetry/telemetry.go Outdated Show resolved Hide resolved

kkumar-gcc added 12 commits December 7, 2025 13:25

add propagators and telemetry interface

d3e238c

add trace sampler

8896f3d

add test cases

7abdb6e

implement telemetry resource

494d812

implement exporters

37174e2

implement application

7d29655

add telemetry module and binding

9496375

add service_provider

4bc034e

add telemetry facade

7453aab

add telemetry setup

8c396a4

change setup package to main

bbfa8c4

update go mod

634ce56

kkumar-gcc requested a review from Copilot December 7, 2025 10:47

Copilot started reviewing on behalf of kkumar-gcc December 7, 2025 10:47 View session

Copilot AI reviewed Dec 7, 2025

View reviewed changes

kkumar-gcc marked this pull request as ready for review December 7, 2025 10:51

kkumar-gcc added 2 commits December 7, 2025 16:29

expose propagator method

5225765

add alias for common methods

212a54b

github-actions bot reviewed Dec 7, 2025

View reviewed changes

kkumar-gcc requested a review from Copilot December 7, 2025 13:39

Copilot started reviewing on behalf of kkumar-gcc December 7, 2025 13:39 View session

Copilot AI reviewed Dec 7, 2025

View reviewed changes

kkumar-gcc commented Dec 7, 2025

View reviewed changes

kkumar-gcc added 3 commits December 7, 2025 19:21

...

163f074

add insecure in grpc

c3d607f

remove unnecessary nil check

d4fbba4

hwbrzzl reviewed Dec 8, 2025

View reviewed changes

	// Supported drivers: "otlp", "zipkin", "console"
	// Supported exporters: "otlp", "zipkin", "console"

	if len(propagators) == 0 {
	return defaultCompositePropagator
	}

feat: [#726][Trace] Add OpenTelemetry Tracing Support for Distributed Tracing #1283

Are you sure you want to change the base?

feat: [#726][Trace] Add OpenTelemetry Tracing Support for Distributed Tracing #1283

Conversation

kkumar-gcc commented Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📑 Description

Summary

Usage

Configuration (.env)

Creating Traces

Context Propagation

Shutdown

Configuration Reference

✅ Checks

Uh oh!

codecov bot commented Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

⚠️ Performance Alert ⚠️

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

Uh oh!

kkumar-gcc Dec 7, 2025 • 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!

Uh oh!

kkumar-gcc Dec 7, 2025 • 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!

hwbrzzl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

kkumar-gcc commented Dec 4, 2025 •

edited

Loading

Configuration (`.env`)

codecov bot commented Dec 4, 2025 •

edited

Loading

kkumar-gcc Dec 7, 2025 •

edited

Loading

kkumar-gcc Dec 7, 2025 •

edited

Loading