Remove SetHttpFlavor from Http instrumentations (#3381)

cijothomas · web-flow · commit d74ac9ea4a29 · 2022-06-18T10:36:27.000-07:00
diff --git a/src/OpenTelemetry.Instrumentation.Http/.publicApi/net462/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Instrumentation.Http/.publicApi/net462/PublicAPI.Unshipped.txt
@@ -6,8 +6,6 @@ OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions.Filter.set -
 OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions.HttpClientInstrumentationOptions() -> void
 OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions.RecordException.get -> bool
 OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions.RecordException.set -> void
-OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions.SetHttpFlavor.get -> bool
-OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions.SetHttpFlavor.set -> void
 OpenTelemetry.Instrumentation.Http.HttpWebRequestInstrumentationOptions
 OpenTelemetry.Instrumentation.Http.HttpWebRequestInstrumentationOptions.Enrich.get -> System.Action<System.Diagnostics.Activity, string, object>
 OpenTelemetry.Instrumentation.Http.HttpWebRequestInstrumentationOptions.Enrich.set -> void
@@ -16,8 +14,6 @@ OpenTelemetry.Instrumentation.Http.HttpWebRequestInstrumentationOptions.Filter.s
 OpenTelemetry.Instrumentation.Http.HttpWebRequestInstrumentationOptions.HttpWebRequestInstrumentationOptions() -> void
 OpenTelemetry.Instrumentation.Http.HttpWebRequestInstrumentationOptions.RecordException.get -> bool
 OpenTelemetry.Instrumentation.Http.HttpWebRequestInstrumentationOptions.RecordException.set -> void
-OpenTelemetry.Instrumentation.Http.HttpWebRequestInstrumentationOptions.SetHttpFlavor.get -> bool
-OpenTelemetry.Instrumentation.Http.HttpWebRequestInstrumentationOptions.SetHttpFlavor.set -> void
 OpenTelemetry.Metrics.MeterProviderBuilderExtensions
 OpenTelemetry.Trace.TracerProviderBuilderExtensions
 static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddHttpClientInstrumentation(this OpenTelemetry.Metrics.MeterProviderBuilder builder) -> OpenTelemetry.Metrics.MeterProviderBuilder
diff --git a/src/OpenTelemetry.Instrumentation.Http/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Instrumentation.Http/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt
@@ -6,8 +6,6 @@ OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions.Filter.set -
 OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions.HttpClientInstrumentationOptions() -> void
 OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions.RecordException.get -> bool
 OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions.RecordException.set -> void
-OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions.SetHttpFlavor.get -> bool
-OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions.SetHttpFlavor.set -> void
 OpenTelemetry.Metrics.MeterProviderBuilderExtensions
 OpenTelemetry.Trace.TracerProviderBuilderExtensions
 static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddHttpClientInstrumentation(this OpenTelemetry.Metrics.MeterProviderBuilder builder) -> OpenTelemetry.Metrics.MeterProviderBuilder
diff --git a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md
@@ -2,6 +2,11 @@
 
 ## Unreleased
 
+* [Breaking] Removes `SetHttpFlavor` option. "http.flavor" is
+  now always automatically populated.
+  To remove this tag, set "http.flavor" to null using `ActivityProcessor`.
+  ([#3380](https://github.com/open-telemetry/opentelemetry-dotnet/issues/3380))
+
 ## 1.0.0-rc9.4
 
 Released 2022-Jun-03
diff --git a/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationOptions.cs b/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationOptions.cs
@@ -19,7 +19,6 @@
 using System.Net.Http;
 using System.Runtime.CompilerServices;
 using OpenTelemetry.Instrumentation.Http.Implementation;
-using OpenTelemetry.Trace;
 
 namespace OpenTelemetry.Instrumentation.Http
 {
@@ -28,11 +27,6 @@ namespace OpenTelemetry.Instrumentation.Http
     /// </summary>
     public class HttpClientInstrumentationOptions
     {
-        /// <summary>
-        /// Gets or sets a value indicating whether or not the HTTP version should be added as the <see cref="SemanticConventions.AttributeHttpFlavor"/> tag. Default value: False.
-        /// </summary>
-        public bool SetHttpFlavor { get; set; }
-
         /// <summary>
         /// Gets or sets a Filter function that determines whether or not to collect telemetry about requests on a per request basis.
         /// The Filter gets the HttpRequestMessage, and should return a boolean.
diff --git a/src/OpenTelemetry.Instrumentation.Http/HttpWebRequestInstrumentationOptions.netfx.cs b/src/OpenTelemetry.Instrumentation.Http/HttpWebRequestInstrumentationOptions.netfx.cs
@@ -19,7 +19,6 @@
 using System.Diagnostics;
 using System.Net;
 using OpenTelemetry.Instrumentation.Http.Implementation;
-using OpenTelemetry.Trace;
 
 namespace OpenTelemetry.Instrumentation.Http
 {
@@ -28,11 +27,6 @@ namespace OpenTelemetry.Instrumentation.Http
     /// </summary>
     public class HttpWebRequestInstrumentationOptions
     {
-        /// <summary>
-        /// Gets or sets a value indicating whether or not the HTTP version should be added as the <see cref="SemanticConventions.AttributeHttpFlavor"/> tag. Default value: False.
-        /// </summary>
-        public bool SetHttpFlavor { get; set; }
-
         /// <summary>
         /// Gets or sets a Filter function that determines whether or not to collect telemetry about requests on a per request basis.
         /// The Filter gets the HttpWebRequest, and should return a boolean.
diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs
@@ -115,10 +115,7 @@ public override void OnStartActivity(Activity activity, object payload)
                 activity.SetTag(SemanticConventions.AttributeHttpMethod, HttpTagHelper.GetNameForHttpMethod(request.Method));
                 activity.SetTag(SemanticConventions.AttributeHttpHost, HttpTagHelper.GetHostTagValueFromRequestUri(request.RequestUri));
                 activity.SetTag(SemanticConventions.AttributeHttpUrl, HttpTagHelper.GetUriTagValueFromRequestUri(request.RequestUri));
-                if (this.options.SetHttpFlavor)
-                {
-                    activity.SetTag(SemanticConventions.AttributeHttpFlavor, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.Version));
-                }
+                activity.SetTag(SemanticConventions.AttributeHttpFlavor, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.Version));
 
                 try
                 {
diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs
@@ -99,10 +99,7 @@ private static void AddRequestTagsAndInstrumentRequest(HttpWebRequest request, A
                 activity.SetTag(SemanticConventions.AttributeHttpMethod, request.Method);
                 activity.SetTag(SemanticConventions.AttributeHttpHost, HttpTagHelper.GetHostTagValueFromRequestUri(request.RequestUri));
                 activity.SetTag(SemanticConventions.AttributeHttpUrl, HttpTagHelper.GetUriTagValueFromRequestUri(request.RequestUri));
-                if (Options.SetHttpFlavor)
-                {
-                    activity.SetTag(SemanticConventions.AttributeHttpFlavor, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.ProtocolVersion));
-                }
+                activity.SetTag(SemanticConventions.AttributeHttpFlavor, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.ProtocolVersion));
 
                 try
                 {
diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.netcore31.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.netcore31.cs
@@ -66,7 +66,6 @@ public async Task HttpOutCallsAreCollectedSuccessfullyAsync(HttpTestData.HttpOut
             using (Sdk.CreateTracerProviderBuilder()
                                .AddHttpClientInstrumentation((opt) =>
                                {
-                                   opt.SetHttpFlavor = tc.SetHttpFlavor;
                                    opt.Enrich = ActivityEnrichment;
                                    opt.RecordException = tc.RecordException ?? false;
                                })
@@ -202,6 +201,7 @@ public async Task DebugIndividualTestAsync()
       ""http.method"": ""GET"",
       ""http.host"": ""{host}:{port}"",
       ""http.status_code"": ""399"",
+      ""http.flavor"": ""2.0"",
       ""http.url"": ""http://{host}:{port}/""
     }
   }
diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpTestData.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpTestData.cs
@@ -74,8 +74,6 @@ public class HttpOutTestCase
             public bool? SpanStatusHasDescription { get; set; }
 
             public Dictionary<string, string> SpanAttributes { get; set; }
-
-            public bool SetHttpFlavor { get; set; }
         }
     }
 }
diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.netfx.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.netfx.cs
@@ -50,7 +50,6 @@ public void HttpOutCallsAreCollectedSuccessfully(HttpTestData.HttpOutTestCase tc
                 .AddProcessor(activityProcessor.Object)
                 .AddHttpClientInstrumentation(options =>
                 {
-                    options.SetHttpFlavor = tc.SetHttpFlavor;
                     options.Enrich = ActivityEnrichment;
                     options.RecordException = tc.RecordException.HasValue ? tc.RecordException.Value : false;
                 })
diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/http-out-test-cases.json b/test/OpenTelemetry.Instrumentation.Http.Tests/http-out-test-cases.json
@@ -1,4 +1,4 @@
-﻿[
+[
   {
     "name": "Successful GET call to localhost",
     "method": "GET",
@@ -9,6 +9,7 @@
     "spanAttributes": {
       "http.method": "GET",
       "http.host": "{host}:{port}",
+      "http.flavor": "2.0",
       "http.status_code": 200,
       "http.url": "http://{host}:{port}/"
     }
@@ -23,6 +24,7 @@
     "spanAttributes": {
       "http.method": "POST",
       "http.host": "{host}:{port}",
+      "http.flavor": "2.0",
       "http.status_code": 200,
       "http.url": "http://{host}:{port}/"
     }
@@ -38,6 +40,7 @@
     "spanAttributes": {
       "http.method": "GET",
       "http.host": "{host}:{port}",
+      "http.flavor": "2.0",
       "http.status_code": 200,
       "http.url": "http://{host}:{port}/path/to/resource/"
     }
@@ -53,6 +56,7 @@
     "spanAttributes": {
       "http.method": "GET",
       "http.host": "{host}:{port}",
+      "http.flavor": "2.0",
       "http.status_code": 200,
       "http.url": "http://{host}:{port}/path/to/resource#fragment"
     }
@@ -68,6 +72,7 @@
     "spanAttributes": {
       "http.method": "GET",
       "http.host": "{host}:{port}",
+      "http.flavor": "2.0",
       "http.status_code": 200,
       "http.url": "http://{host}:{port}/path/to/resource#fragment"
     }
@@ -84,6 +89,7 @@
     "spanAttributes": {
       "http.method": "GET",
       "http.host": "sdlfaldfjalkdfjlkajdflkajlsdjf.sdlkjafsdjfalfadslkf.com",
+      "http.flavor": "2.0",
       "http.url": "https://sdlfaldfjalkdfjlkajdflkajlsdjf.sdlkjafsdjfalfadslkf.com/"
     }
   },
@@ -99,6 +105,7 @@
     "spanAttributes": {
       "http.method": "GET",
       "http.host": "sdlfaldfjalkdfjlkajdflkajlsdjf.sdlkjafsdjfalfadslkf.com",
+      "http.flavor": "2.0",
       "http.url": "https://sdlfaldfjalkdfjlkajdflkajlsdjf.sdlkjafsdjfalfadslkf.com/"
     }
   },
@@ -113,6 +120,7 @@
     "spanAttributes": {
       "http.method": "GET",
       "http.host": "{host}:{port}",
+      "http.flavor": "2.0",
       "http.status_code": 200,
       "http.url": "http://{host}:{port}/"
     }
@@ -128,6 +136,7 @@
     "spanAttributes": {
       "http.method": "GET",
       "http.host": "{host}:{port}",
+      "http.flavor": "2.0",
       "http.status_code": 200,
       "http.url": "http://{host}:{port}/"
     }
@@ -143,6 +152,7 @@
     "spanAttributes": {
       "http.method": "GET",
       "http.host": "{host}:{port}",
+      "http.flavor": "2.0",
       "http.status_code": "399",
       "http.url": "http://{host}:{port}/"
     }
@@ -158,6 +168,7 @@
     "spanAttributes": {
       "http.method": "GET",
       "http.host": "{host}:{port}",
+      "http.flavor": "2.0",
       "http.status_code": "400",
       "http.url": "http://{host}:{port}/"
     }
@@ -173,6 +184,7 @@
     "spanAttributes": {
       "http.method": "GET",
       "http.host": "{host}:{port}",
+      "http.flavor": "2.0",
       "http.status_code": "401",
       "http.url": "http://{host}:{port}/"
     }
@@ -188,6 +200,7 @@
     "spanAttributes": {
       "http.method": "GET",
       "http.host": "{host}:{port}",
+      "http.flavor": "2.0",
       "http.status_code": "403",
       "http.url": "http://{host}:{port}/"
     }
@@ -203,6 +216,7 @@
     "spanAttributes": {
       "http.method": "GET",
       "http.host": "{host}:{port}",
+      "http.flavor": "2.0",
       "http.status_code": "404",
       "http.url": "http://{host}:{port}/"
     }
@@ -218,6 +232,7 @@
     "spanAttributes": {
       "http.method": "GET",
       "http.host": "{host}:{port}",
+      "http.flavor": "2.0",
       "http.status_code": "429",
       "http.url": "http://{host}:{port}/"
     }
@@ -233,6 +248,7 @@
     "spanAttributes": {
       "http.method": "GET",
       "http.host": "{host}:{port}",
+      "http.flavor": "2.0",
       "http.status_code": "501",
       "http.url": "http://{host}:{port}/"
     }
@@ -248,6 +264,7 @@
     "spanAttributes": {
       "http.method": "GET",
       "http.host": "{host}:{port}",
+      "http.flavor": "2.0",
       "http.status_code": "503",
       "http.url": "http://{host}:{port}/"
     }
@@ -263,6 +280,7 @@
     "spanAttributes": {
       "http.method": "GET",
       "http.host": "{host}:{port}",
+      "http.flavor": "2.0",
       "http.status_code": "504",
       "http.url": "http://{host}:{port}/"
     }
@@ -278,6 +296,7 @@
     "spanAttributes": {
       "http.method": "GET",
       "http.host": "{host}:{port}",
+      "http.flavor": "2.0",
       "http.status_code": "600",
       "http.url": "http://{host}:{port}/"
     }
@@ -290,7 +309,6 @@
     "spanName": "HTTP GET",
     "spanStatus": "UNSET",
     "responseExpected": true,
-    "setHttpFlavor": true,
     "spanAttributes": {
       "http.method": "GET",
       "http.host": "{host}:{port}",

Original file line number	Diff line number	Diff line change
`@@ -66,7 +66,6 @@ public async Task HttpOutCallsAreCollectedSuccessfullyAsync(HttpTestData.HttpOut`
`66`	`66`	`using (Sdk.CreateTracerProviderBuilder()`
`67`	`67`	`.AddHttpClientInstrumentation((opt) =>`
`68`	`68`	`{`
`69`		`- opt.SetHttpFlavor = tc.SetHttpFlavor;`
`70`	`69`	`opt.Enrich = ActivityEnrichment;`
`71`	`70`	`opt.RecordException = tc.RecordException ?? false;`
`72`	`71`	`})`
`@@ -202,6 +201,7 @@ public async Task DebugIndividualTestAsync()`
`202`	`201`	`""http.method"": ""GET"",`
`203`	`202`	`""http.host"": ""{host}:{port}"",`
`204`	`203`	`""http.status_code"": ""399"",`
	`204`	`+ ""http.flavor"": ""2.0"",`
`205`	`205`	`""http.url"": ""http://{host}:{port}/""`
`206`	`206`	`}`
`207`	`207`	`}`
Original file line number	Diff line number	Diff line change
`@@ -74,8 +74,6 @@ public class HttpOutTestCase`
`74`	`74`	`public bool? SpanStatusHasDescription { get; set; }`
`75`	`75`
`76`	`76`	`public Dictionary<string, string> SpanAttributes { get; set; }`
`77`		`-`
`78`		`- public bool SetHttpFlavor { get; set; }`
`79`	`77`	`}`
`80`	`78`	`}`
`81`	`79`	`}`
Original file line number	Diff line number	Diff line change
`@@ -50,7 +50,6 @@ public void HttpOutCallsAreCollectedSuccessfully(HttpTestData.HttpOutTestCase tc`
`50`	`50`	`.AddProcessor(activityProcessor.Object)`
`51`	`51`	`.AddHttpClientInstrumentation(options =>`
`52`	`52`	`{`
`53`		`- options.SetHttpFlavor = tc.SetHttpFlavor;`
`54`	`53`	`options.Enrich = ActivityEnrichment;`
`55`	`54`	`options.RecordException = tc.RecordException.HasValue ? tc.RecordException.Value : false;`
`56`	`55`	`})`