Repository: open-telemetry/opentelemetry-go-contrib
Pull request #3343 (2023-01-30 03:06:38)

add span name generator for otelgin option

linux-foundation-easycla[bot]:  <a href="https://api.easycla.lfx.linuxfoundation.org/v2/repository-provider/github/sign/9522512/242202166/3343/#/?version=2"><img src="https://s3.amazonaws.com/cla-project-logo-prod/cla-not-signed.svg" alt="CLA Not Signed" align="left" height="28" width="328"></a><br/><br /><ul><li><a href='https://api.easycla.lfx.linuxfoundation.org/v2/repository-provider/github/sign/9522512/242202166/3343/#/?version=2' target='_blank'>:x:</a> - login: @jojotong . The commit (a5f2428d551b349c1433decf707879f8bfc690aa) is not authorized under a signed CLA. <a href='https://api.easycla.lfx.linuxfoundation.org/v2/repository-provider/github/sign/9522512/242202166/3343/#/?version=2' target='_blank'>Please click here to be authorized</a>. For further assistance with EasyCLA, <a href='https://jira.linuxfoundation.org/servicedesk/customer/portal/4' target='_blank'>please submit a support request ticket</a>.</li></ul>
codecov[bot]: # [Codecov](https://codecov.io/gh/open-telemetry/opentelemetry-go-contrib/pull/3343?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry) Report
> Merging [#3343](https://codecov.io/gh/open-telemetry/opentelemetry-go-contrib/pull/3343?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry) (a5f2428) into [main](https://codecov.io/gh/open-telemetry/opentelemetry-go-contrib/commit/10abc596595411001ccbf2d9d278a64d884abd16?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry) (10abc59) will **decrease** coverage by `0.1%`.
> The diff coverage is `27.2%`.

<details><summary>Additional details and impacted files</summary>


[![Impacted file tree graph](https://codecov.io/gh/open-telemetry/opentelemetry-go-contrib/pull/3343/graphs/tree.svg?width=650&height=150&src=pr&token=P6F3W9WA7Q&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry)](https://codecov.io/gh/open-telemetry/opentelemetry-go-contrib/pull/3343?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry)

```diff
@@           Coverage Diff           @@
##            main   #3343     +/-   ##
=======================================
- Coverage   69.5%   69.4%   -0.1%     
=======================================
  Files        147     147             
  Lines       6890    6900     +10     
=======================================
+ Hits        4789    4791      +2     
- Misses      1980    1987      +7     
- Partials     121     122      +1     
```


| [Impacted Files](https://codecov.io/gh/open-telemetry/opentelemetry-go-contrib/pull/3343?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry) | Coverage Δ | |
|---|---|---|
| [...ntation/github.com/gin-gonic/gin/otelgin/option.go](https://codecov.io/gh/open-telemetry/opentelemetry-go-contrib/pull/3343?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry#diff-aW5zdHJ1bWVudGF0aW9uL2dpdGh1Yi5jb20vZ2luLWdvbmljL2dpbi9vdGVsZ2luL29wdGlvbi5nbw==) | `76.1% <0.0%> (-23.9%)` | :arrow_down: |
| [...ation/github.com/gin-gonic/gin/otelgin/gintrace.go](https://codecov.io/gh/open-telemetry/opentelemetry-go-contrib/pull/3343?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry#diff-aW5zdHJ1bWVudGF0aW9uL2dpdGh1Yi5jb20vZ2luLWdvbmljL2dpbi9vdGVsZ2luL2dpbnRyYWNlLmdv) | `80.2% <50.0%> (-2.4%)` | :arrow_down: |

</details>
dmathieu: I think this should be named WithSpanNameFormatter, to match the net/http config.
https://github.com/open-telemetry/opentelemetry-go-contrib/blob/main/instrumentation/net/http/otelhttp/config.go#L189
jojotong: > I think this should be named WithSpanNameFormatter, to match the net/http config.

Thanks for notice, I've changed to to match the net/http config
dmathieu: @jojotong could you fix the tests, lint and add a changelog entry?
jojotong: @dmathieu fixed.
jojotong: Rebased to resolve conflicts.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 29232bfe5..7cd951f3a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 - Add the new `go.opentelemetry.io/contrib/instrgen` package to provide auto-generated source code instrumentation. (#3068)
 - Support [Go 1.20]. (#3372)
+- Add `SpanNameFormatter` option to package `go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin`. (#3343)
 
 ### Changed
 
diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go
index feeb13fc9..6e445967b 100644
--- a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go
+++ b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go
@@ -73,7 +73,12 @@ func Middleware(service string, opts ...Option) gin.HandlerFunc {
 			oteltrace.WithAttributes(httpconv.ServerRequest(service, c.Request)...),
 			oteltrace.WithSpanKind(oteltrace.SpanKindServer),
 		}
-		spanName := c.FullPath()
+		var spanName string
+		if cfg.SpanNameFormatter == nil {
+			spanName = c.FullPath()
+		} else {
+			spanName = cfg.SpanNameFormatter(c.Request)
+		}
 		if spanName == "" {
 			spanName = fmt.Sprintf("HTTP %s route not found", c.Request.Method)
 		} else {
diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/option.go b/instrumentation/github.com/gin-gonic/gin/otelgin/option.go
index d8e1d1d9e..c13c3dad8 100644
--- a/instrumentation/github.com/gin-gonic/gin/otelgin/option.go
+++ b/instrumentation/github.com/gin-gonic/gin/otelgin/option.go
@@ -24,15 +24,19 @@ import (
 )
 
 type config struct {
-	TracerProvider oteltrace.TracerProvider
-	Propagators    propagation.TextMapPropagator
-	Filters        []Filter
+	TracerProvider    oteltrace.TracerProvider
+	Propagators       propagation.TextMapPropagator
+	Filters           []Filter
+	SpanNameFormatter SpanNameFormatter
 }
 
 // Filter is a predicate used to determine whether a given http.request should
 // be traced. A Filter must return true if the request should be traced.
 type Filter func(*http.Request) bool
 
+// SpanNameFormatter is used to set span name by http.request.
+type SpanNameFormatter func(r *http.Request) string
+
 // Option specifies instrumentation configuration options.
 type Option interface {
 	apply(*config)
@@ -76,3 +80,11 @@ func WithFilter(f ...Filter) Option {
 		c.Filters = append(c.Filters, f...)
 	})
 }
+
+// WithSpanNameFormatter takes a function that will be called on every
+// request and the returned string will become the Span Name.
+func WithSpanNameFormatter(f func(r *http.Request) string) Option {
+	return optionFunc(func(c *config) {
+		c.SpanNameFormatter = f
+	})
+}


Test patch:
diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/test/gintrace_test.go b/instrumentation/github.com/gin-gonic/gin/otelgin/test/gintrace_test.go
index e84772f20..184a03713 100644
--- a/instrumentation/github.com/gin-gonic/gin/otelgin/test/gintrace_test.go
+++ b/instrumentation/github.com/gin-gonic/gin/otelgin/test/gintrace_test.go
@@ -164,6 +164,32 @@ func TestSpanStatus(t *testing.T) {
 	}
 }
 
+func TestSpanName(t *testing.T) {
+	testCases := []struct {
+		requestPath       string
+		spanNameFormatter otelgin.SpanNameFormatter
+		wantSpanName      string
+	}{
+		{"/user/1", nil, "/user/:id"},
+		{"/user/1", func(r *http.Request) string { return r.URL.Path }, "/user/1"},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.requestPath, func(t *testing.T) {
+			sr := tracetest.NewSpanRecorder()
+			provider := sdktrace.NewTracerProvider()
+			provider.RegisterSpanProcessor(sr)
+			router := gin.New()
+			router.Use(otelgin.Middleware("foobar", otelgin.WithTracerProvider(provider), otelgin.WithSpanNameFormatter(tc.spanNameFormatter)))
+			router.GET("/user/:id", func(c *gin.Context) {})
+
+			router.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", tc.requestPath, nil))
+
+			require.Len(t, sr.Ended(), 1, "should emit a span")
+			assert.Equal(t, sr.Ended()[0].Name(), tc.wantSpanName, "span name not correct")
+		})
+	}
+}
+
 func TestHTML(t *testing.T) {
 	sr := tracetest.NewSpanRecorder()
 	provider := sdktrace.NewTracerProvider(sdktrace.WithSpanProcessor(sr))