Fix view bugs where views create conflicting metric streams at runtime

[alanwest]

This PR reopens #3074. It is an alternative to #3006.

In this one, views that create conflicts at runtime are ignored.

[codecov]

Codecov Report

Merging #3129 (7a9a450) into main (d3c0c6e) will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #3129      +/-   ##
==========================================
+ Coverage   84.74%   84.80%   +0.05%     
==========================================
  Files         259      259              
  Lines        9236     9246      +10     
==========================================
+ Hits         7827     7841      +14     
+ Misses       1409     1405       -4     

Impacted Files	Coverage Δ
src/OpenTelemetry/Metrics/InstrumentIdentity.cs	91.42% <100.00%> (+1.10%)	⬆️
src/OpenTelemetry/Metrics/MetricReaderExt.cs	86.61% <100.00%> (+0.66%)	⬆️
...emetry.Api/Internal/OpenTelemetryApiEventSource.cs	73.52% <0.00%> (-8.83%)	⬇️
...ZPages/Implementation/ZPagesExporterEventSource.cs	56.25% <0.00%> (-6.25%)	⬇️
...nTelemetry/Internal/OpenTelemetrySdkEventSource.cs	76.72% <0.00%> (+2.58%)	⬆️
...lemetry/Internal/SelfDiagnosticsConfigRefresher.cs	89.42% <0.00%> (+2.88%)	⬆️
...tation/OpenTelemetryProtocolExporterEventSource.cs	85.00% <0.00%> (+10.00%)	⬆️

[reyang]

LGTM.

[cijothomas]

Nit: move these tests to the MetricViewTests.cs

[cijothomas]

yes.

[alanwest]

Moved and added a test for this scenario 462d1af

[cijothomas]

This log must only be inside the if (metrics.Count==0) part.

[alanwest]

On second thought, I removed this log message. I think it is unnecessary. It is not an issue of a duplicate conflicting metric instrument. The if (metrics.Count == 0) case is meant to handle instruments mapping to identical metric streams (i.e., same name, kind, description, etc) - so it is safe to aggregate together.

For example, this test covers this case

opentelemetry-dotnet/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs

Lines 503 to 524 in 3bf47f8

	[Fact]
	public void DuplicateInstrumentRegistration_WithViews_TwoIdenticalInstruments_TwoViews_DifferentTags()
	{
	var exportedItems = new List<Metric>();
	using var meter = new Meter($"{Utils.GetCurrentMethodName()}");
	var meterProviderBuilder = Sdk.CreateMeterProviderBuilder()
	.AddMeter(meter.Name)
	.AddView((instrument) =>
	{
	return new MetricStreamConfiguration { TagKeys = new[] { "key1" } };
	})
	.AddView((instrument) =>
	{
	return new MetricStreamConfiguration { TagKeys = new[] { "key2" } };
	})
	.AddInMemoryExporter(exportedItems);
	using var meterProvider = meterProviderBuilder.Build();
	var instrument1 = meter.CreateCounter<long>("name");
	var instrument2 = meter.CreateCounter<long>("name");

Both instrument1 and instrument2 match the first view, the metric stream is identical, so they can be aggregated together. The second view is ignored, and a log message is produced in this case.

---

Update: Actually this log message does still belong. I incorrectly moved it. Moved it back per your comment here #3129 (comment)

[cijothomas]

we still need this check, and warning, for the following test

[Fact]
        public void Test()
        {
            var exportedItems = new List<Metric>();

            using var meter = new Meter($"{Utils.GetCurrentMethodName()}");
            var meterProviderBuilder = Sdk.CreateMeterProviderBuilder()
                .AddMeter(meter.Name)
                .AddView("uncommon", MetricStreamConfiguration.Drop)
                .AddInMemoryExporter(exportedItems);

            using var meterProvider = meterProviderBuilder.Build();

            var instrument1 = meter.CreateCounter<long>("name", description: "desc1");
            var instrument2 = meter.CreateCounter<long>("name", description: "desc2");

            instrument1.Add(10);
            instrument2.Add(10);

            meterProvider.ForceFlush(MaxTimeToAllowForFlush);

            Assert.Equal(2, exportedItems.Count);
            Assert.Equal("name", exportedItems[0].Name);
            Assert.Equal("desc1", exportedItems[0].Description);
            Assert.Equal("name", exportedItems[1].Name);
            Assert.Equal("desc2", exportedItems[1].Description);
        }

[alanwest]

We've settled on #3006 as that is the direction the spec will be taking