[translator/zipkin] stop dropping error tags in Zipkin translator (op…

…en-telemetry#24547)

**Description:** <Describe what has changed.>
Fixing a bug - Stop dropping error tags from Zipkin spans. The old code
removes all errors from those spans, rendering them useless if an actual
error happened. In addition, no longer delete error tags if they contain
useful information. This now only deletes them if they are "true", and
thus don't contain useful information. Also fixes the whole issue of

**Link to tracking Issue:** open-telemetry#16530

**Testing:** Absolutely none, I have written no Go code before this and
have not even run the code.

I'm hoping CI saves me the trouble of installing and running everything
locally

---------

Co-authored-by: Curtis Robert <crobert@splunk.com>
Co-authored-by: Curtis Robert <92119472+crobert-1@users.noreply.github.com>
Co-authored-by: Antoine Toulme <antoine@lunar-ocean.com>

.chloggen/zipking-translator-dropped-errors.yaml

@@ -0,0 +1,20 @@
+# Use this changelog template to create an entry for release notes.
+# If your change doesn't affect end users, such as a test fix or a tooling change,
+# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: bug_fix
+
+# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
+component: zipkintranslator
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Stop dropping error tags from Zipkin spans. The old code removes all errors from those spans, rendering them useless if an actual error happened. In addition, no longer delete error tags if they contain useful information.
+
+# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
+issues: [16530]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:

pkg/translator/zipkin/zipkinv2/to_translator.go

@@ -155,8 +155,8 @@ func populateSpanStatus(tags map[string]string, status ptrace.Status) {
 	}
 
 	if val, ok := tags[tracetranslator.TagError]; ok {
+		status.SetCode(ptrace.StatusCodeError)
 		if val == "true" {
-			status.SetCode(ptrace.StatusCodeError)
 			delete(tags, tracetranslator.TagError)
 		}
 	}

receiver/zipkinreceiver/testdata/sample3.json

@@ -0,0 +1,33 @@
+[
+  {
+    "traceId": "4d1e00c0db9010db86154a4ba6e91385",
+    "parentId": "86154a4ba6e91385",
+    "id": "4d1e00c0db9010db",
+    "kind": "CLIENT",
+    "name": "get",
+    "timestamp": 1472470996199000,
+    "duration": 207000,
+    "localEndpoint": {
+      "ipv6": "7::0.128.128.127"
+    },
+    "remoteEndpoint": {
+      "ipv4": "192.168.99.101",
+      "port": 9000
+    },
+    "annotations": [
+      {
+        "timestamp": 1472470996238000,
+        "value": "foo"
+      },
+      {
+        "timestamp": 1472470996403000,
+        "value": "bar"
+      }
+    ],
+    "tags": {
+      "http.path": "/api",
+      "clnt/finagle.version": "6.45.0",
+      "error": "true"
+    }
+  }
+]

receiver/zipkinreceiver/testdata/sample4.json

@@ -0,0 +1,33 @@
+[
+  {
+    "traceId": "4d1e00c0db9010db86154a4ba6e91385",
+    "parentId": "86154a4ba6e91385",
+    "id": "4d1e00c0db9010db",
+    "kind": "CLIENT",
+    "name": "get",
+    "timestamp": 1472470996199000,
+    "duration": 207000,
+    "localEndpoint": {
+      "ipv6": "7::0.128.128.127"
+    },
+    "remoteEndpoint": {
+      "ipv4": "192.168.99.101",
+      "port": 9000
+    },
+    "annotations": [
+      {
+        "timestamp": 1472470996238000,
+        "value": "foo"
+      },
+      {
+        "timestamp": 1472470996403000,
+        "value": "bar"
+      }
+    ],
+    "tags": {
+      "http.path": "/api",
+      "clnt/finagle.version": "6.45.0",
+      "error": "Non-basic error message"
+    }
+  }
+]

receiver/zipkinreceiver/trace_receiver_test.go

@@ -26,6 +26,7 @@ import (
 	"go.opentelemetry.io/collector/config/confighttp"
 	"go.opentelemetry.io/collector/consumer"
 	"go.opentelemetry.io/collector/consumer/consumertest"
+	"go.opentelemetry.io/collector/pdata/ptrace"
 	"go.opentelemetry.io/collector/receiver/receivertest"
 	conventions "go.opentelemetry.io/collector/semconv/v1.6.1"
 )
@@ -352,6 +353,44 @@ func TestConvertSpansToTraceSpans_JSONWithoutSerivceName(t *testing.T) {
 	require.Equal(t, 1, reqs.SpanCount(), "Incorrect non-nil spans count")
 }
 
+func TestConvertSpansToTraceSpans_JSONWithSimpleError(t *testing.T) {
+	blob, err := os.ReadFile("./testdata/sample3.json")
+	require.NoError(t, err, "Failed to read sample JSON file: %v", err)
+	zi := newTestZipkinReceiver()
+	reqs, err := zi.v2ToTraceSpans(blob, nil)
+	require.NoError(t, err, "Failed to parse convert Zipkin spans in JSON to Trace spans: %v", err)
+
+	require.Equal(t, 1, reqs.ResourceSpans().Len(), "Expecting only one request since all spans share same node/localEndpoint: %v", reqs.ResourceSpans().Len())
+
+	// Expecting 1 non-nil spans
+	require.Equal(t, 1, reqs.SpanCount(), "Incorrect non-nil spans count")
+
+	require.Equal(t, ptrace.StatusCodeError, reqs.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0).Status().Code())
+
+	_, exists := reqs.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0).Attributes().Get("error")
+	require.False(t, exists, "Error attribute should be removed when error is simply set to \"true\".")
+}
+
+func TestConvertSpansToTraceSpans_JSONWithErrorMessage(t *testing.T) {
+	blob, err := os.ReadFile("./testdata/sample4.json")
+	require.NoError(t, err, "Failed to read sample JSON file: %v", err)
+	zi := newTestZipkinReceiver()
+	reqs, err := zi.v2ToTraceSpans(blob, nil)
+	require.NoError(t, err, "Failed to parse convert Zipkin spans in JSON to Trace spans: %v", err)
+
+	require.Equal(t, 1, reqs.ResourceSpans().Len(), "Expecting only one request since all spans share same node/localEndpoint: %v", reqs.ResourceSpans().Len())
+
+	// Expecting 1 non-nil spans
+	require.Equal(t, 1, reqs.SpanCount(), "Incorrect non-nil spans count")
+
+	require.Equal(t, ptrace.StatusCodeError, reqs.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0).Status().Code(),
+		"Error code should be set to the proper error status since the trace had an error tag.")
+
+	errorMessage, exists := reqs.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0).Attributes().Get("error")
+	require.True(t, exists, "Given span should have an error attribute with the received span's error message.")
+	require.Equal(t, "Non-basic error message", errorMessage.AsString(), "Error message should be retained in the span.")
+}
+
 func TestReceiverConvertsStringsToTypes(t *testing.T) {
 	body, err := os.ReadFile(zipkinV2Single)
 	require.NoError(t, err, "Failed to read sample JSON file: %v", err)