From 14e552b8000cac38a4d974362a1fcc8b5ac0956f Mon Sep 17 00:00:00 2001
From: Tyler Yahn <codingalias@gmail.com>
Date: Tue, 18 Jul 2023 10:14:00 -0700
Subject: [PATCH] Detect dup inst for case-insensitive names

Resolve #3835

Detect duplicate instrument registrations for instruments that have the
same case-insensitive names. Continue to return the instruments with
different names, but log a warning. This is the solution proposed in
https://github.com/open-telemetry/opentelemetry-specification/pull/3606.
---
 sdk/metric/pipeline.go      |  5 ++-
 sdk/metric/pipeline_test.go | 70 +++++++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/sdk/metric/pipeline.go b/sdk/metric/pipeline.go
index 05fe29ce4f9..9fe48bcdeba 100644
--- a/sdk/metric/pipeline.go
+++ b/sdk/metric/pipeline.go
@@ -351,7 +351,10 @@ func (i *inserter[N]) cachedAggregator(scope instrumentation.Scope, kind Instrum
 // logConflict validates if an instrument with the same name as id has already
 // been created. If that instrument conflicts with id, a warning is logged.
 func (i *inserter[N]) logConflict(id streamID) {
-	existing := i.views.Lookup(id.Name, func() streamID { return id })
+	// The API specification defines names as case-insensitive. If there is a
+	// different casing of a name it needs to be a conflict.
+	name := strings.ToLower(id.Name)
+	existing := i.views.Lookup(name, func() streamID { return id })
 	if id == existing {
 		return
 	}
diff --git a/sdk/metric/pipeline_test.go b/sdk/metric/pipeline_test.go
index f9056275c47..511febf535f 100644
--- a/sdk/metric/pipeline_test.go
+++ b/sdk/metric/pipeline_test.go
@@ -17,12 +17,15 @@ package metric // import "go.opentelemetry.io/otel/sdk/metric"
 import (
 	"context"
 	"fmt"
+	"strings"
 	"sync"
 	"testing"
 
+	"github.com/go-logr/logr/funcr"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/sdk/instrumentation"
 	"go.opentelemetry.io/otel/sdk/metric/metricdata"
@@ -166,3 +169,70 @@ func testDefaultViewImplicit[N int64 | float64]() func(t *testing.T) {
 		}
 	}
 }
+
+func TestLogConflictName(t *testing.T) {
+	testcases := []struct {
+		existing, name string
+		conflict       bool
+	}{
+		{
+			existing: "requestCount",
+			name:     "requestCount",
+			conflict: false,
+		},
+		{
+			existing: "requestCount",
+			name:     "requestDuration",
+			conflict: false,
+		},
+		{
+			existing: "requestCount",
+			name:     "requestcount",
+			conflict: true,
+		},
+		{
+			existing: "requestCount",
+			name:     "REQUESTCOUNT",
+			conflict: true,
+		},
+		{
+			existing: "requestCount",
+			name:     "rEqUeStCoUnT",
+			conflict: true,
+		},
+	}
+
+	var msg string
+	otel.SetLogger(funcr.New(func(_, args string) {
+		msg = args
+	}, funcr.Options{Verbosity: 20}))
+
+	for _, tc := range testcases {
+		var vc cache[string, streamID]
+
+		name := strings.ToLower(tc.existing)
+		_ = vc.Lookup(name, func() streamID {
+			return streamID{Name: tc.existing}
+		})
+
+		i := newInserter[int64](newPipeline(nil, nil, nil), &vc)
+		i.logConflict(streamID{Name: tc.name})
+
+		if tc.conflict {
+			assert.Containsf(
+				t, msg, "duplicate metric stream definitions",
+				"warning not logged for conflicting names: %s, %s",
+				tc.existing, tc.name,
+			)
+		} else {
+			assert.Equalf(
+				t, msg, "",
+				"warning logged for non-conflicting names: %s, %s",
+				tc.existing, tc.name,
+			)
+		}
+
+		// Reset.
+		msg = ""
+	}
+}