SqlClient Instrumentation to leverage native Activity Status. (#3751)

Yun-Ting · web-flow · commit ee9b892d3223 · 2022-10-12T15:42:35.000-04:00
diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md
@@ -2,6 +2,12 @@
 
 ## Unreleased
 
+* Use `Activity.Status` and `Activity.StatusDescription` properties instead of
+  `OpenTelemetry.Trace.Status` and `OpenTelemetry.Trace.Status.Description`
+  respectively to set activity status.
+  ([#3118](https://github.com/open-telemetry/opentelemetry-dotnet/issues/3118))
+  ([#3751](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3751))
+
 ## 1.0.0-rc9.7
 
 Released 2022-Sep-29
diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs
@@ -146,17 +146,7 @@ public override void OnEventWritten(string name, object payload)
                             return;
                         }
 
-                        try
-                        {
-                            if (activity.IsAllDataRequested)
-                            {
-                                activity.SetStatus(Status.Unset);
-                            }
-                        }
-                        finally
-                        {
-                            activity.Stop();
-                        }
+                        activity.Stop();
                     }
 
                     break;
@@ -180,7 +170,7 @@ public override void OnEventWritten(string name, object payload)
                             {
                                 if (this.exceptionFetcher.TryFetch(payload, out Exception exception) && exception != null)
                                 {
-                                    activity.SetStatus(Status.Error.WithDescription(exception.Message));
+                                    activity.SetStatus(ActivityStatusCode.Error, exception.Message);
 
                                     if (this.options.RecordException)
                                     {
diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlEventSourceListener.netfx.cs b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlEventSourceListener.netfx.cs
@@ -181,18 +181,17 @@ private void OnEndExecute(EventWrittenEventArgs eventData)
                 if (activity.IsAllDataRequested)
                 {
                     int compositeState = (int)eventData.Payload[1];
-                    if ((compositeState & 0b001) == 0b001)
+                    if ((compositeState & 0b001) != 0b001)
                     {
-                        activity.SetStatus(Status.Unset);
-                    }
-                    else if ((compositeState & 0b010) == 0b010)
-                    {
-                        var errorText = $"SqlExceptionNumber {eventData.Payload[2]} thrown.";
-                        activity.SetStatus(Status.Error.WithDescription(errorText));
-                    }
-                    else
-                    {
-                        activity.SetStatus(Status.Error.WithDescription("Unknown Sql failure."));
+                        if ((compositeState & 0b010) == 0b010)
+                        {
+                            var errorText = $"SqlExceptionNumber {eventData.Payload[2]} thrown.";
+                            activity.SetStatus(ActivityStatusCode.Error, errorText);
+                        }
+                        else
+                        {
+                            activity.SetStatus(ActivityStatusCode.Error, "Unknown Sql failure.");
+                        }
                     }
                 }
             }
diff --git a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTests.cs b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTests.cs
@@ -326,13 +326,13 @@ private static void VerifyActivityData(
 
             if (!isFailure)
             {
-                Assert.Equal(Status.Unset, activity.GetStatus());
+                Assert.Equal(ActivityStatusCode.Unset, activity.Status);
             }
             else
             {
                 var status = activity.GetStatus();
-                Assert.Equal(Status.Error.StatusCode, status.StatusCode);
-                Assert.NotNull(status.Description);
+                Assert.Equal(ActivityStatusCode.Error, activity.Status);
+                Assert.NotNull(activity.StatusDescription);
 
                 if (recordException)
                 {
diff --git a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlEventSourceTests.netfx.cs b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlEventSourceTests.netfx.cs
@@ -276,13 +276,12 @@ private static void VerifyActivityData(
 
             if (!isFailure)
             {
-                Assert.Equal(Status.Unset, activity.GetStatus());
+                Assert.Equal(ActivityStatusCode.Unset, activity.Status);
             }
             else
             {
-                var status = activity.GetStatus();
-                Assert.Equal(Status.Error.StatusCode, status.StatusCode);
-                Assert.NotNull(status.Description);
+                Assert.Equal(ActivityStatusCode.Error, activity.Status);
+                Assert.NotNull(activity.StatusDescription);
             }
         }
 

Original file line number	Diff line number	Diff line change
`@@ -146,17 +146,7 @@ public override void OnEventWritten(string name, object payload)`
`146`	`146`	`return;`
`147`	`147`	`}`
`148`	`148`
`149`		`- try`
`150`		`- {`
`151`		`- if (activity.IsAllDataRequested)`
`152`		`- {`
`153`		`- activity.SetStatus(Status.Unset);`
`154`		`- }`
`155`		`- }`
`156`		`- finally`
`157`		`- {`
`158`		`- activity.Stop();`
`159`		`- }`
	`149`	`+ activity.Stop();`
`160`	`150`	`}`
`161`	`151`
`162`	`152`	`break;`
`@@ -180,7 +170,7 @@ public override void OnEventWritten(string name, object payload)`
`180`	`170`	`{`
`181`	`171`	`if (this.exceptionFetcher.TryFetch(payload, out Exception exception) && exception != null)`
`182`	`172`	`{`
`183`		`- activity.SetStatus(Status.Error.WithDescription(exception.Message));`
	`173`	`+ activity.SetStatus(ActivityStatusCode.Error, exception.Message);`
`184`	`174`
`185`	`175`	`if (this.options.RecordException)`
`186`	`176`	`{`
Original file line number	Diff line number	Diff line change
`@@ -181,18 +181,17 @@ private void OnEndExecute(EventWrittenEventArgs eventData)`
`181`	`181`	`if (activity.IsAllDataRequested)`
`182`	`182`	`{`
`183`	`183`	`int compositeState = (int)eventData.Payload[1];`
`184`		`- if ((compositeState & 0b001) == 0b001)`
	`184`	`+ if ((compositeState & 0b001) != 0b001)`
`185`	`185`	`{`
`186`		`- activity.SetStatus(Status.Unset);`
`187`		`- }`
`188`		`- else if ((compositeState & 0b010) == 0b010)`
`189`		`- {`
`190`		`- var errorText = $"SqlExceptionNumber {eventData.Payload[2]} thrown.";`
`191`		`- activity.SetStatus(Status.Error.WithDescription(errorText));`
`192`		`- }`
`193`		`- else`
`194`		`- {`
`195`		`- activity.SetStatus(Status.Error.WithDescription("Unknown Sql failure."));`
	`186`	`+ if ((compositeState & 0b010) == 0b010)`
	`187`	`+ {`
	`188`	`+ var errorText = $"SqlExceptionNumber {eventData.Payload[2]} thrown.";`
	`189`	`+ activity.SetStatus(ActivityStatusCode.Error, errorText);`
	`190`	`+ }`
	`191`	`+ else`
	`192`	`+ {`
	`193`	`+ activity.SetStatus(ActivityStatusCode.Error, "Unknown Sql failure.");`
	`194`	`+ }`
`196`	`195`	`}`
`197`	`196`	`}`
`198`	`197`	`}`
Original file line number	Diff line number	Diff line change
`@@ -326,13 +326,13 @@ private static void VerifyActivityData(`
`326`	`326`
`327`	`327`	`if (!isFailure)`
`328`	`328`	`{`
`329`		`- Assert.Equal(Status.Unset, activity.GetStatus());`
	`329`	`+ Assert.Equal(ActivityStatusCode.Unset, activity.Status);`
`330`	`330`	`}`
`331`	`331`	`else`
`332`	`332`	`{`
`333`	`333`	`var status = activity.GetStatus();`
`334`		`- Assert.Equal(Status.Error.StatusCode, status.StatusCode);`
`335`		`- Assert.NotNull(status.Description);`
	`334`	`+ Assert.Equal(ActivityStatusCode.Error, activity.Status);`
	`335`	`+ Assert.NotNull(activity.StatusDescription);`
`336`	`336`
`337`	`337`	`if (recordException)`
`338`	`338`	`{`
Original file line number	Diff line number	Diff line change
`@@ -276,13 +276,12 @@ private static void VerifyActivityData(`
`276`	`276`
`277`	`277`	`if (!isFailure)`
`278`	`278`	`{`
`279`		`- Assert.Equal(Status.Unset, activity.GetStatus());`
	`279`	`+ Assert.Equal(ActivityStatusCode.Unset, activity.Status);`
`280`	`280`	`}`
`281`	`281`	`else`
`282`	`282`	`{`
`283`		`- var status = activity.GetStatus();`
`284`		`- Assert.Equal(Status.Error.StatusCode, status.StatusCode);`
`285`		`- Assert.NotNull(status.Description);`
	`283`	`+ Assert.Equal(ActivityStatusCode.Error, activity.Status);`
	`284`	`+ Assert.NotNull(activity.StatusDescription);`
`286`	`285`	`}`
`287`	`286`	`}`
`288`	`287`