Add kind in otelriver river.work traces name

[quantumsheep]

Instead of one group of traces with the same name, it's more comfortable to have different groups per job

[brandur]

Nice!

Just a couple things:

• I think we should probably leave in the option for work spans to all be grouped together and differentiated by attribute (the job kind is also an attribute) instead of span name, so this should probably be a config option rather than the only default.
• Is there a reason to prefer a forward slash ("/") in the name as opposed to the dots you normally see? i.e. Should this be something like river.work.my_job instead of river.work/my_job? I don't mind the look of the slash, but based on what I've seen, dots might be more conventional.

I put together a small patch here that's roughly what I'm envisioning for the first point:

diff --git a/otelriver/middleware.go b/otelriver/middleware.go
index f778470..be7e10b 100644
--- a/otelriver/middleware.go
+++ b/otelriver/middleware.go
@@ -44,6 +44,10 @@ type MiddlewareConfig struct {
 	// metric names, with attributes differentiating them.
 	EnableSemanticMetrics bool
 
+	// EnableWorkSpanJobKindSuffix appends the job kind a suffix to work spans
+	// so they look like `river.work/my_job` instead of `river.work`.
+	EnableWorkSpanJobKindSuffix bool
+
 	// MeterProvider is a MeterProvider to base metrics on. May be left as nil
 	// to use the default global provider.
 	MeterProvider metric.MeterProvider
@@ -179,7 +183,12 @@ func (m *Middleware) InsertMany(ctx context.Context, manyParams []*rivertype.Job
 }
 
 func (m *Middleware) Work(ctx context.Context, job *rivertype.JobRow, doInner func(context.Context) error) error {
-	ctx, span := m.tracer.Start(ctx, prefix+"work/"+job.Kind,
+	spanName := prefix + "work"
+	if m.config.EnableWorkSpanJobKindSuffix {
+		spanName += "/" + job.Kind
+	}
+
+	ctx, span := m.tracer.Start(ctx, spanName,
 		trace.WithSpanKind(trace.SpanKindConsumer))
 	defer span.End()
 
diff --git a/otelriver/middleware_test.go b/otelriver/middleware_test.go
index f73a8d1..a0b84e3 100644
--- a/otelriver/middleware_test.go
+++ b/otelriver/middleware_test.go
@@ -295,7 +295,7 @@ func TestMiddleware(t *testing.T) {
 		require.Equal(t, "ok", getAttribute(t, span.Attributes, "status").AsString())
 		require.Equal(t, scheduledAt.Format(time.RFC3339), getAttribute(t, span.Attributes, "scheduled_at").AsString())
 		require.Equal(t, []string{"a", "b"}, getAttribute(t, span.Attributes, "tag").AsStringSlice())
-		require.Equal(t, "river.work/no_op", span.Name)
+		require.Equal(t, "river.work", span.Name)
 		require.Equal(t, codes.Ok, span.Status.Code)
 
 		var (
@@ -344,7 +344,7 @@ func TestMiddleware(t *testing.T) {
 		require.Equal(t, int64(6), getAttribute(t, span.Attributes, "attempt").AsInt64())
 		require.Equal(t, "my_queue", getAttribute(t, span.Attributes, "queue").AsString())
 		require.Equal(t, "error", getAttribute(t, span.Attributes, "status").AsString())
-		require.Equal(t, "river.work/no_op", span.Name)
+		require.Equal(t, "river.work", span.Name)
 		require.Equal(t, codes.Error, span.Status.Code)
 		require.Equal(t, "error from doInner", span.Status.Description)
 
@@ -426,7 +426,7 @@ func TestMiddleware(t *testing.T) {
 		require.Equal(t, int64(6), getAttribute(t, span.Attributes, "attempt").AsInt64())
 		require.Equal(t, "my_queue", getAttribute(t, span.Attributes, "queue").AsString())
 		require.Equal(t, "panic", getAttribute(t, span.Attributes, "status").AsString())
-		require.Equal(t, "river.work/no_op", span.Name)
+		require.Equal(t, "river.work", span.Name)
 		require.Equal(t, codes.Error, span.Status.Code)
 		require.Equal(t, "panic", span.Status.Description)
 
@@ -517,6 +517,30 @@ func TestMiddleware(t *testing.T) {
 			require.Equal(t, "s", metric.Unit)
 		}
 	})
+
+	t.Run("WorkEnableWorkSpanJobKindSuffix ", func(t *testing.T) {
+		t.Parallel()
+
+		middleware, bundle := setupConfig(t, &MiddlewareConfig{
+			EnableWorkSpanJobKindSuffix: true,
+		})
+
+		doInner := func(ctx context.Context) error {
+			return nil
+		}
+
+		err := middleware.Work(ctx, &rivertype.JobRow{
+			ID:   123,
+			Kind: "no_op",
+		}, doInner)
+		require.NoError(t, err)
+
+		spans := bundle.traceExporter.GetSpans()
+		require.Len(t, spans, 1)
+
+		span := spans[0]
+		require.Equal(t, "river.work/no_op", span.Name)
+	})
 }
 
 func getAttribute(t *testing.T, attrs []attribute.KeyValue, key string) attribute.Value {

[brandur]

Also, just while I have you here, would you mind "signing" (adding your username to a file) the River CLA?

https://github.com/riverqueue/rivercla

[quantumsheep]

Here you go! riverqueue/rivercla#12

[quantumsheep]

I like your patch 👍 The reasoning behind the / was that kinds are custom values. Spaces would not fit in the . format:

# With / format
river.work/My Super Job

# With . format
river.work.My Super Job

What do you think?

[brandur]

Yeah fair point on the possibility of spaces. Alright, LGTM.

I pulled your commit into #23 and added my option on top. Going to close this one in favor of that.

[brandur]

@quantumsheep Just cut v0.4.0.