feat: [#726][Trace] Add OpenTelemetry Tracing Support for Distributed Tracing by krishankumar01 · Pull Request #1283 · goravel/framework

krishankumar01 · 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 59.05797% with 113 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.97%. Comparing base (52c3e24) to head (2f14cdb).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
telemetry/setup/stubs.go	0.00%	55 Missing ⚠️
telemetry/setup/setup.go	0.00%	19 Missing ⚠️
telemetry/service_provider.go	0.00%	17 Missing ⚠️
foundation/container.go	0.00%	6 Missing ⚠️
telemetry/alias.go	0.00%	4 Missing ⚠️
telemetry/application.go	92.30%	3 Missing and 1 partial ⚠️
telemetry/exporter.go	93.93%	2 Missing and 2 partials ⚠️
telemetry/propagator.go	90.47%	1 Missing and 1 partial ⚠️
telemetry/resource.go	90.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1283      +/-   ##
==========================================
- Coverage   69.14%   68.97%   -0.18%     
==========================================
  Files         265      275      +10     
  Lines       15671    15947     +276     
==========================================
+ Hits        10836    10999     +163     
- Misses       4368     4476     +108     
- Partials      467      472       +5

☔ 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: `ed119d1`	Previous: `9a4a7b4`	Ratio
`Benchmark_DecryptString`	`6361` ns/op 2032 B/op 16 allocs/op	`2082` ns/op 2032 B/op 16 allocs/op	`3.06`
`Benchmark_DecryptString - ns/op`	`6361` ns/op	`2082` ns/op	`3.06`

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

hwbrzzl

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

telemetry/setup/setup.go

telemetry/setup/stubs.go

telemetry/propagator.go

telemetry/application.go

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.

Yeah, that’s why I experimented a bit with the propagator and exporter, you can check that they both expect a config struct. We can follow the same approach, and our service provider should resolve this config struct. If you agree, I can start moving at least this module to that philosophy. Or we can expose a method UnmarshalKey to map a key to a go struct.

@hwbrzzl I have added a config.go we can follow a similar way to resolve all config

Yes, use the module's own config instead of facades.Config directly. It's easier to mock.

krishankumar01 · 2025-12-08T05:17:25Z

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

Yeah, I’ll implement auto-instrumentation in later PRs.

hwbrzzl · 2025-12-09T08:04:38Z

telemetry/setup/setup.go

+	telemetryServiceProvider := "&telemetry.ServiceProvider{}"
+	modulePath := packages.GetModulePath()
+
+	packages.Setup(os.Args).


I optimized the setup process. Currently, packages.Setup(os.Args) should be called first to initiate Paths from os.Args. Otherwise, path.Config("telemetry.go") will be incorrect if user customizes the config path.

I think we need to rethink the paths feature. In the code you pass a configPackage, for example:

WithPaths(func(paths configuration.Paths) { paths.Config("hello") paths.Facades("new") })

If a user customises paths like this, when they install the cache package it should create the config in the hello directory and the facades in the new directory. But your current stubs only customise moduleName in the path. I’m not sure that’s the right approach, it looks like we need to optimise this before adopting it.

Let’s optimise this in a later PR.

Yes, I'm optimizing it.

hwbrzzl · 2025-12-09T08:10:30Z

telemetry/setup/stubs.go

+				"timeout":         config.Env("OTEL_EXPORTER_OTLP_TIMEOUT", 10000),
+				"traces_timeout":  config.Env("OTEL_EXPORTER_OTLP_TRACES_TIMEOUT", ""),


Got it. Do we have a plan to add metrics and logs later?

hwbrzzl

Awesome, LGTM.

… Tracing (#1283) * init telemetry contracts package * add propagators and telemetry interface * add trace sampler * add test cases * implement telemetry resource * implement exporters * implement application * add telemetry module and binding * add service_provider * add telemetry facade * add telemetry setup * change setup package to main * update go mod * expose propagator method * add alias for common methods * ... * add insecure in grpc * remove unnecessary nil check * make propagator mandatory * optimise telemetry setup * optimise telemetry application * optimise telemetry application * use config struct instead of using config facade * remove support/config * optimise setup.go * optimise telemetry/setup * optimise telemetry/setup

init telemetry contracts package

0acd121

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

krishankumar01 requested a review from a team as a code owner December 4, 2025 13:56

krishankumar01 marked this pull request as draft December 4, 2025 13:56

Copilot started reviewing on behalf of krishankumar01 December 4, 2025 13:56 View session

krishankumar01 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

krishankumar01 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 krishankumar01 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

krishankumar01 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

krishankumar01 requested a review from Copilot December 7, 2025 10:47

Copilot started reviewing on behalf of krishankumar01 December 7, 2025 10:47 View session

Copilot AI reviewed Dec 7, 2025

View reviewed changes

krishankumar01 marked this pull request as ready for review December 7, 2025 10:51

krishankumar01 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

krishankumar01 requested a review from Copilot December 7, 2025 13:39

Copilot started reviewing on behalf of krishankumar01 December 7, 2025 13:39 View session

Copilot AI reviewed Dec 7, 2025

View reviewed changes

krishankumar01 commented Dec 7, 2025

View reviewed changes

support/config/key.go Outdated Show resolved Hide resolved

krishankumar01 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

krishankumar01 added 7 commits December 8, 2025 12:14

make propagator mandatory

4b1609e

optimise telemetry setup

4c6b681

optimise telemetry application

a9fae73

optimise telemetry application

a3f4c5d

Merge branch 'master' into kkumar-gcc/#726

5a97ab6

use config struct instead of using config facade

64c4e7a

remove support/config

ed119d1

hwbrzzl reviewed Dec 9, 2025

View reviewed changes

krishankumar01 and others added 4 commits December 9, 2025 17:25

Merge branch 'master' into kkumar-gcc/#726

929d714

optimise setup.go

1f12acc

optimise telemetry/setup

ad4e55b

optimise telemetry/setup

2f14cdb

hwbrzzl approved these changes Dec 9, 2025

View reviewed changes

krishankumar01 merged commit 18d63ba into master Dec 9, 2025
12 of 14 checks passed

krishankumar01 deleted the kkumar-gcc/#726 branch December 9, 2025 12:43

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

		"timeout": config.Env("OTEL_EXPORTER_OTLP_TIMEOUT", 10000),
		"traces_timeout": config.Env("OTEL_EXPORTER_OTLP_TRACES_TIMEOUT", ""),

Conversation

krishankumar01 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 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

krishankumar01 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!

Uh oh!

hwbrzzl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krishankumar01 Dec 8, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krishankumar01 commented Dec 8, 2025

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!

krishankumar01 commented Dec 4, 2025 •

edited

Loading

Configuration (`.env`)

codecov bot commented Dec 4, 2025 •

edited

Loading

github-actions bot left a comment •

edited

Loading

krishankumar01 Dec 7, 2025 •

edited

Loading

krishankumar01 Dec 8, 2025 •

edited

Loading