Log a view suggestion for duplicate instrument conflicts

CHANGELOG.md

@@ -43,6 +43,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Fix `resource.WithHostID()` to not set an empty `host.id`. (#4317)
 - Use the instrument identifying fields to cache aggregators and determine duplicate instrument registrations in `go.opentelemetry.io/otel/sdk/metric`. (#4337)
 - Detect duplicate instruments for case-insensitive names in `go.opentelemetry.io/otel/sdk/metric`. (#4338)
+- Suggest a view to fix fixable instrument conflicts in `go.opentelemetry.io/otel/sdk/metric`. (#4349)
 
 ## [1.16.0/0.39.0] 2023-05-18
 

sdk/metric/pipeline.go

@@ -361,14 +361,39 @@ func (i *inserter[N]) logConflict(id instID) {
 		return
 	}
 
-	global.Warn(
-		"duplicate metric stream definitions",
+	const msg = "duplicate metric stream definitions"
+	args := []interface{}{
 		"names", fmt.Sprintf("%q, %q", existing.Name, id.Name),
 		"descriptions", fmt.Sprintf("%q, %q", existing.Description, id.Description),
 		"kinds", fmt.Sprintf("%s, %s", existing.Kind, id.Kind),
 		"units", fmt.Sprintf("%s, %s", existing.Unit, id.Unit),
 		"numbers", fmt.Sprintf("%s, %s", existing.Number, id.Number),
+	}
+
+	// The specification recommends logging a suggested view to resolve
+	// conflicts if possible.
+	//
+	// https://github.com/open-telemetry/opentelemetry-specification/blob/v1.21.0/specification/metrics/sdk.md#duplicate-instrument-registration
+	if id.Unit != existing.Unit || id.Number != existing.Number {
+		// There is no view resolution for these, don't make a suggestion.
+		global.Warn(msg, args...)
+		return
+	}
+
+	var stream string
+	if id.Name != existing.Name || id.Kind != existing.Kind {
+		stream = `Stream{Name: "{{NEW_NAME}}"}`
+	} else if id.Description != existing.Description {
+		stream = fmt.Sprintf("Stream{Description: %q}", existing.Description)
+	}
+
+	inst := fmt.Sprintf(
+		"Instrument{Name: %q, Description: %q, Kind: %q, Unit: %q}",
+		id.Name, id.Description, "InstrumentKind"+id.Kind.String(), id.Unit,
 	)
+	args = append(args, "suggested.view", fmt.Sprintf("NewView(%s, %s)", inst, stream))
+
+	global.Warn(msg, args...)
 }
 
 func (i *inserter[N]) instID(kind InstrumentKind, stream Stream) instID {

sdk/metric/pipeline_test.go

@@ -243,3 +243,118 @@ func TestLogConflictName(t *testing.T) {
 		msg = ""
 	}
 }
+
+func TestLogConflictSuggestView(t *testing.T) {
+	var msg string
+	t.Cleanup(func(orig logr.Logger) func() {
+		otel.SetLogger(funcr.New(func(_, args string) {
+			msg = args
+		}, funcr.Options{Verbosity: 20}))
+		return func() { otel.SetLogger(orig) }
+	}(stdr.New(log.New(os.Stderr, "", log.LstdFlags|log.Lshortfile))))
+
+	orig := instID{
+		Name:        "requestCount",
+		Description: "number of requests",
+		Kind:        InstrumentKindCounter,
+		Unit:        "1",
+		Number:      "int64",
+	}
+
+	var vc cache[string, instID]
+	name := strings.ToLower(orig.Name)
+	_ = vc.Lookup(name, func() instID { return orig })
+	i := newInserter[int64](newPipeline(nil, nil, nil), &vc)
+
+	viewSuggestion := func(inst instID, stream string) string {
+		return `"NewView(Instrument{` +
+			`Name: \"` + inst.Name +
+			`\", Description: \"` + inst.Description +
+			`\", Kind: \"InstrumentKind` + inst.Kind.String() +
+			`\", Unit: \"` + inst.Unit +
+			`\"}, ` +
+			stream +
+			`)"`
+	}
+
+	t.Run("Name", func(t *testing.T) {
+		inst := instID{
+			Name:        "requestcount",
+			Description: orig.Description,
+			Kind:        orig.Kind,
+			Unit:        orig.Unit,
+			Number:      orig.Number,
+		}
+		i.logConflict(inst)
+		assert.Containsf(t, msg, viewSuggestion(
+			inst, `Stream{Name: \"{{NEW_NAME}}\"}`,
+		), "no suggestion logged: %v", inst)
+
+		// Reset.
+		msg = ""
+	})
+
+	t.Run("Description", func(t *testing.T) {
+		inst := instID{
+			Name:        orig.Name,
+			Description: "alt",
+			Kind:        orig.Kind,
+			Unit:        orig.Unit,
+			Number:      orig.Number,
+		}
+		i.logConflict(inst)
+		assert.Containsf(t, msg, viewSuggestion(
+			inst, `Stream{Description: \"`+orig.Description+`\"}`,
+		), "no suggestion logged: %v", inst)
+
+		// Reset.
+		msg = ""
+	})
+
+	t.Run("Kind", func(t *testing.T) {
+		inst := instID{
+			Name:        orig.Name,
+			Description: orig.Description,
+			Kind:        InstrumentKindHistogram,
+			Unit:        orig.Unit,
+			Number:      orig.Number,
+		}
+		i.logConflict(inst)
+		assert.Containsf(t, msg, viewSuggestion(
+			inst, `Stream{Name: \"{{NEW_NAME}}\"}`,
+		), "no suggestion logged: %v", inst)
+
+		// Reset.
+		msg = ""
+	})
+
+	t.Run("Unit", func(t *testing.T) {
+		inst := instID{
+			Name:        orig.Name,
+			Description: orig.Description,
+			Kind:        orig.Kind,
+			Unit:        "ms",
+			Number:      orig.Number,
+		}
+		i.logConflict(inst)
+		assert.NotContains(t, msg, "NewView", "suggestion logged: %v", inst)
+
+		// Reset.
+		msg = ""
+	})
+
+	t.Run("Number", func(t *testing.T) {
+		inst := instID{
+			Name:        orig.Name,
+			Description: orig.Description,
+			Kind:        orig.Kind,
+			Unit:        orig.Unit,
+			Number:      "float64",
+		}
+		i.logConflict(inst)
+		assert.NotContains(t, msg, "NewView", "suggestion logged: %v", inst)
+
+		// Reset.
+		msg = ""
+	})
+}