feat(gin): add gin-gonic/gin server instrumentation by waqar2403 · Pull Request #501 · open-telemetry/opentelemetry-go-compile-instrumentation

waqar2403 · 2026-05-13T10:02:42Z

Description

Adds compile-time instrumentation for gin-gonic/gin HTTP servers using a single-hook enrichment approach.

Instead of creating a separate span, we hook (*gin.Context).Next and enrich the span already created by the net/http server hook. By the time Next is called, gin's router has matched the request and populated c.FullPath(), so we update the span name from "METHOD" to "METHOD /route/pattern" and record the http.route attribute.

This keeps the telemetry output aligned with otelgin conventions: same span name format, same route attribute, one span per request.

Key design decisions:

Single hook only: net/http's serverHandler.ServeHTTP already fires for gin (since gin.Engine implements http.Handler), so no need for a second span-creation hook
Idempotency guard: c.Next() is called by each middleware in the chain, so the hook uses c.Set/c.Get to ensure the span is only updated once per request
Empty route handling: If no route matched (404), the span name stays as just the HTTP method

Resolves #479

Checklist

PR title follows conventional commits format
Code formatted: make format
Linters pass: make lint
Tests pass: make test
Tests added for new functionality
Tests follow testing guidelines
Documentation updated (if applicable)

Single-hook approach: hook (*Context).Next to enrich the span already created by the net/http server hook. Sets span name to "METHOD /route" and records the http.route attribute, aligned with otelgin conventions. Includes unit tests, test app, and e2e tests.

Move gin_test.go from test/e2e/ to test/integration/gin_server_test.go and update the build tag from e2e to integration, since the test exercises a single instrumented process (not multiple services). Follows the convention in docs/testing.md and matches the existing integration tests for http, grpc, redis, and db_client.

NameHaibinZhang

Review Summary

Nice approach — enriching the existing net/http span instead of creating a duplicate is the right call. The idempotency guard via c.Set/c.Get handles the multi-middleware c.Next() problem cleanly.

Issues to address

Integration test hardcodes port 8080 — Unlike http_server_test.go which parameterizes the port, the gin test uses a fixed port. This can cause flaky failures if the port is occupied. Suggest passing -port=<unused> like other tests:
```
f.BuildAndStart("ginserver", "-port=8090")
```
No integration test for the 404/no-route case — The unit test covers empty route, but an end-to-end test verifying that a request to an unregistered path still produces a span named just "GET" (without route pattern) would strengthen regression protection.
Heavy transitive dependency tree — The go.mod pulls in prometheus, grpc-gateway, autoexport, and many OTLP exporters indirectly. For a hook that only needs otel/trace + otel/semconv, this seems excessive. This may be inherited from shared/pkg and not specific to this PR, but worth investigating whether the dependency graph can be trimmed.

Nits

PR scope in title: feat(instrumentation) is broad — feat(gin) would be more specific.
Consider documenting "otel.gin.route.set" as a reserved key in gin context, to avoid potential collision with user middleware.

Overall

Solid contribution. The design is sound, tests cover the key scenarios, and it aligns well with otelgin conventions. Addressing the hardcoded port should be straightforward. 👍

Copilot

Pull request overview

Adds compile-time instrumentation support for github.com/gin-gonic/gin servers by hooking (*gin.Context).Next to enrich the existing net/http server span with a route-based span name and http.route attribute, aligning output with common otelgin conventions.

Changes:

Introduces a Gin server instrumentation module with a Context.Next hook that updates span name and sets http.route.
Adds unit tests for the hook behavior (route enrichment, empty route no-op, idempotency, enable/disable).
Adds an integration test app (ginserver) plus integration tests validating span naming/attributes for route patterns and 5xx errors.

Reviewed changes

Copilot reviewed 8 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
test/integration/gin_server_test.go	New integration tests asserting route-template span naming and `http.route`/status/error attributes.
test/apps/ginserver/main.go	Minimal Gin server app used by integration tests.
test/apps/ginserver/go.mod	Module definition for the Gin test app.
test/apps/ginserver/go.sum	Dependency lockfile for the Gin test app.
pkg/instrumentation/gin/server/server.yaml	Adds the compile-time hook rule for `(*gin.Context).Next`.
pkg/instrumentation/gin/server/go.mod	Module definition for the Gin instrumentation package.
pkg/instrumentation/gin/server/go.sum	Dependency lockfile for the Gin instrumentation package.
pkg/instrumentation/gin/server/gin.go	Defines the Gin instrumentation enablement key and logger.
pkg/instrumentation/gin/server/context_hook.go	Implements `BeforeNext` hook to set span name and `http.route`.
pkg/instrumentation/gin/server/context_hook_test.go	Unit tests covering span enrichment and idempotency behavior.

Comments suppressed due to low confidence (2)

pkg/instrumentation/gin/server/context_hook_test.go:90

Repository docs/code expect OTEL_GO_ENABLED_INSTRUMENTATIONS values to be lowercase (the list is lowercased during parsing). For consistency with other tests and documentation, prefer using "gin" instead of "GIN" here.

	sr, tr := setupContextTracer(t)
	t.Setenv("OTEL_GO_ENABLED_INSTRUMENTATIONS", "GIN")

pkg/instrumentation/gin/server/context_hook_test.go:111

Repository docs/code expect OTEL_GO_ENABLED_INSTRUMENTATIONS values to be lowercase (the list is lowercased during parsing). For consistency with other tests and documentation, prefer using "gin" instead of "GIN" here.

	sr, tr := setupContextTracer(t)
	t.Setenv("OTEL_GO_ENABLED_INSTRUMENTATIONS", "GIN")

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

+	// c.Next() is called by each middleware in the chain, so this hook fires
+	// multiple times per request. Only the first call needs to update the span.
+	if _, already := c.Get(routeSetKey); already {
+		return
+	}
+	c.Set(routeSetKey, struct{}{})
+
+	span := trace.SpanFromContext(c.Request.Context())
+	if !span.IsRecording() {
+		return
+	}


+
+func TestBeforeNext_UpdatesSpanNameAndRoute(t *testing.T) {
+	sr, tr := setupContextTracer(t)
+	t.Setenv("OTEL_GO_ENABLED_INSTRUMENTATIONS", "GIN")


+
+func TestBeforeNext_DisabledIsNoop(t *testing.T) {
+	sr, tr := setupContextTracer(t)
+	t.Setenv("OTEL_GO_DISABLED_INSTRUMENTATIONS", "GIN")


…ions Set routeSetKey only after the IsRecording check passes. Previously a non-recording first call would burn the gate and prevent a later recording span on the same request from being enriched. Switch test env values for OTEL_GO_{ENABLED,DISABLED}_INSTRUMENTATIONS to lowercase 'gin', matching the convention documented on shared.Instrumented and used by the nethttp, grpc, and redis tests. Pass -port=8090 to ginserver in the integration tests so the port is explicit at the call site, matching http_server_test.go. Add a regression test that exercises the non-recording-then-recording path to lock in the gate ordering.

waqar2403 · 2026-05-14T22:50:32Z

Thanks @NameHaibinZhang for the review. Here are the fixes that I have made.

Reordered the gate in BeforeNext so routeSetKey is set only after the IsRecording check passes. Earlier a non recording first call would burn the gate and block any later recording span on the same request from being enriched. Rare in practice but added TestBeforeNext_NonRecordingSpanDoesNotBurnGate to lock it in.
Changed the four OTEL_GO_ENABLED_INSTRUMENTATIONS and OTEL_GO_DISABLED_INSTRUMENTATIONS values from "GIN" to "gin" to match the doc on shared.Instrumented and the nethttp grpc redis tests. Both work since parsing lowercases anyway, this is just for consistency.
Passing -port=8090 to ginserver in the integration test instead of relying on the default 8080. Same pattern as http_server_test.go.
Updated the godoc on routeSetKey to mark it as reserved so user middleware does not collide. The "otel." prefix already signalled it but explicit is better.
Renamed the PR title to feat(gin) as suggested.
Added TestGinServer_UnregisteredRoute for the no route case. It hits an unregistered path and asserts the span name stays as plain GET http.route is not set and url.path is the literal path. Mainly a cardinality guard so a future regression cannot turn every scanner hit into a unique span name.
On the dependency tree I am looking into that separately and will share my findings once done.

let me know if anything else needs change.

codecov · 2026-05-15T08:21:02Z

Codecov Report

❌ Patch coverage is 86.95652% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.52%. Comparing base (260b04d) to head (9588ee0).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/instrumentation/gin/server/context_hook.go	86.36%	3 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #501      +/-   ##
==========================================
+ Coverage   63.30%   63.52%   +0.22%     
==========================================
  Files          62       64       +2     
  Lines        4796     4842      +46     
==========================================
+ Hits         3036     3076      +40     
- Misses       1509     1512       +3     
- Partials      251      254       +3

Flag	Coverage Δ
pkg	`63.52% <86.95%> (+0.22%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

txabman42 · 2026-05-15T11:59:26Z

+  target: github.com/gin-gonic/gin
+  func: Next
+  recv: "*Context"
+  before: BeforeNext


Could be interesting to have an AfterNext to catch all the c.Errors(), similar to go-contrib approach

Hi @txabman42
Added AfterNext hook that captures c.Errors() after (*gin.Context).Next returns — sets span status to Error and records each error as an exception event via span.RecordError(), matching the otelgin approach. Uses the same flag-based idempotency as BeforeNext to handle multiple middleware calling c.Next(). Also added a /error route to the test app and TestGinServer_GinError integration test that exercises c.Error() on a 200 OK response to verify gin-level errors are captured independently of HTTP status.

As you correctly did in the BeforeNext, we need to handle in AfterNext the case where multiple middlewares call c.Next().

We can count in the BeforeNext the number of middlewares and use it to iterate over all errors in the outer middleware.

Switched from the flag based approach to a depth counter as you suggested. BeforeNext increments the counter for each c.Next() call and AfterNext decrements it, only recording errors when the count reaches zero at the outermost middleware return. This way errors added by middleware after c.Next() returns are also captured.

linux-foundation-easycla · 2026-05-15T15:27:41Z

The committers listed above are authorized under a signed CLA.

✅ login: waqar2403 / name: Muhammad Waqar (7539da2, ad4e4da)
✅ login: waqar2403 / name: waqar2403 (047b2c6, 338424a, 510768d, ebb7144, 181462c, f1140d1, f279b0e)

waqar2403 · 2026-05-18T05:11:18Z

@txabman42 I have improved the gin integration tests so they share a single build of the test app instead of building it 4 times. This saves about 90 seconds on the Windows job and keeps the suite well inside the ten minute timeout.

txabman42

LGTM! ❤️

Just a minor nitpick

txabman42 · 2026-05-19T08:17:31Z

+)
+
+func TestGinServer(t *testing.T) {
+	build := testutil.NewTestFixture(t, testutil.WithoutCollector())


No needed, f := testutil.NewTestFixture(t) is done inside each run

txabman42 · 2026-05-19T08:40:02Z

+	"github.com/open-telemetry/opentelemetry-go-compile-instrumentation/test/testutil"
+)
+
+func TestGinServer(t *testing.T) {


Regarding integration test fail:

Couple of ideas to evaluate:

We are not seeing why the app crashed, it seems cmd.Stdout is nil (defined in Start).

Why can verify if the port is already binded during start. If so, we can improve the cleanup phase so it doesn't happen or use different ports.

Thanks for helping. I need a bit of time to figure out the right approach. Will push an update soon.

waqar2403 requested a review from a team as a code owner May 13, 2026 10:02

github-actions Bot added the scope:feat A new feature being added label May 13, 2026

waqar2403 force-pushed the feat/gin-instrumentation branch from d73eb0b to ebb7144 Compare May 13, 2026 10:04

waqar2403 and others added 3 commits May 13, 2026 15:11

Merge branch 'main' into feat/gin-instrumentation

ad4e4da

test(gin): assert single span on error path

338424a

NameHaibinZhang requested review from NameHaibinZhang and Copilot May 14, 2026 08:32

Copilot started reviewing on behalf of NameHaibinZhang May 14, 2026 08:33 View session

NameHaibinZhang reviewed May 14, 2026

View reviewed changes

Copilot AI reviewed May 14, 2026

View reviewed changes

waqar2403 and others added 4 commits May 15, 2026 02:34

Merge branch 'main' into feat/gin-instrumentation

7539da2

docs(gin): mark routeSetKey as reserved on gin context

181462c

test(gin): add integration test for unmatched route

f1140d1

waqar2403 changed the title ~~feat(instrumentation): add gin-gonic/gin server instrumentation~~ feat(gin): add gin-gonic/gin server instrumentation May 14, 2026

txabman42 requested changes May 15, 2026

View reviewed changes

waqar2403 force-pushed the feat/gin-instrumentation branch from c97a127 to f1140d1 Compare May 15, 2026 15:23

feat(gin): add AfterNext hook to record gin context errors

510768d

waqar2403 force-pushed the feat/gin-instrumentation branch from 2b2329c to 510768d Compare May 15, 2026 15:43

waqar2403 added 2 commits May 16, 2026 00:52

use depth counter to capture errors from all middleware layers

f48eaf6

test(gin): share one ginserver build across subtests

a94c40d

waqar2403 force-pushed the feat/gin-instrumentation branch from 00c61ad to a94c40d Compare May 18, 2026 04:48

Merge branch 'main' into feat/gin-instrumentation

0921e16

txabman42 approved these changes May 19, 2026

View reviewed changes

txabman42 requested a review from NameHaibinZhang May 19, 2026 08:26

txabman42 requested changes May 19, 2026

View reviewed changes

Merge branch 'main' into feat/gin-instrumentation

9588ee0

kakkoyun self-requested a review May 19, 2026 11:47

Conversation

waqar2403 commented May 13, 2026

Description

Checklist

Uh oh!

NameHaibinZhang left a comment

Choose a reason for hiding this comment

Review Summary

Issues to address

Nits

Overall

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

waqar2403 commented May 14, 2026

Uh oh!

codecov Bot commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

txabman42 May 15, 2026

Choose a reason for hiding this comment

Uh oh!

waqar2403 May 15, 2026

Choose a reason for hiding this comment

Uh oh!

txabman42 May 15, 2026

Choose a reason for hiding this comment

Uh oh!

waqar2403 May 15, 2026

Choose a reason for hiding this comment

Uh oh!

linux-foundation-easycla Bot commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

waqar2403 commented May 18, 2026

Uh oh!

txabman42 left a comment

Choose a reason for hiding this comment

Uh oh!

txabman42 May 19, 2026

Choose a reason for hiding this comment

Uh oh!

txabman42 May 19, 2026

Choose a reason for hiding this comment

Uh oh!

waqar2403 May 19, 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov Bot commented May 15, 2026 •

edited

Loading

linux-foundation-easycla Bot commented May 15, 2026 •

edited

Loading

waqar2403 May 19, 2026 •

edited

Loading