Merge branch 'main' into zipkin-span-id-uniqify

cmd/query/app/apiv3/http_gateway.go

@@ -135,10 +135,26 @@ func (h *HTTPGateway) getTrace(w http.ResponseWriter, r *http.Request) {
 	if h.tryParamError(w, err, paramTraceID) {
 		return
 	}
-	// TODO: add start time & end time
 	request := spanstore.GetTraceParameters{
 		TraceID: traceID,
 	}
+	http_query := r.URL.Query()
+	startTime := http_query.Get(paramStartTime)
+	if startTime != "" {
+		timeParsed, err := time.Parse(time.RFC3339Nano, startTime)
+		if h.tryParamError(w, err, paramStartTime) {
+			return
+		}
+		request.StartTime = timeParsed.UTC()
+	}
+	endTime := http_query.Get(paramEndTime)
+	if endTime != "" {
+		timeParsed, err := time.Parse(time.RFC3339Nano, endTime)
+		if h.tryParamError(w, err, paramEndTime) {
+			return
+		}
+		request.EndTime = timeParsed.UTC()
+	}
 	trc, err := h.QueryService.GetTrace(r.Context(), request)
 	if h.tryHandleError(w, err, http.StatusInternalServerError) {
 		return

cmd/query/app/apiv3/http_gateway_test.go

@@ -18,6 +18,7 @@ import (
 	"go.uber.org/zap"
 
 	"github.com/jaegertracing/jaeger/cmd/query/app/querysvc"
+	"github.com/jaegertracing/jaeger/model"
 	"github.com/jaegertracing/jaeger/pkg/jtracer"
 	"github.com/jaegertracing/jaeger/pkg/testutils"
 	"github.com/jaegertracing/jaeger/storage/spanstore"
@@ -90,27 +91,111 @@ func TestHTTPGatewayTryHandleError(t *testing.T) {
 	assert.Contains(t, string(w.Body.String()), e, "writes error message to body")
 }
 
-func TestHTTPGatewayGetTraceErrors(t *testing.T) {
-	gw := setupHTTPGatewayNoServer(t, "")
+func TestHTTPGatewayGetTrace(t *testing.T) {
+	traceId, _ := model.TraceIDFromString("123")
+	testCases := []struct {
+		name          string
+		params        map[string]string
+		expectedQuery spanstore.GetTraceParameters
+	}{
+		{
+			name:   "TestGetTrace",
+			params: map[string]string{},
+			expectedQuery: spanstore.GetTraceParameters{
+				TraceID: traceId,
+			},
+		},
+		{
+			name: "TestGetTraceWithTimeWindow",
+			params: map[string]string{
+				"start_time": "2000-01-02T12:30:08.999999998Z",
+				"end_time":   "2000-04-05T21:55:16.999999992+08:00",
+			},
+			expectedQuery: spanstore.GetTraceParameters{
+				TraceID:   traceId,
+				StartTime: time.Date(2000, time.January, 0o2, 12, 30, 8, 999999998, time.UTC),
+				EndTime:   time.Date(2000, time.April, 0o5, 13, 55, 16, 999999992, time.UTC),
+			},
+		},
+	}
 
-	// malformed trace id
-	r, err := http.NewRequest(http.MethodGet, "/api/v3/traces/xyz", nil)
-	require.NoError(t, err)
-	w := httptest.NewRecorder()
-	gw.router.ServeHTTP(w, r)
-	assert.Contains(t, w.Body.String(), "malformed parameter trace_id")
+	testUri := "/api/v3/traces/123"
 
-	// error from span reader
-	const simErr = "simulated error"
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			gw := setupHTTPGatewayNoServer(t, "")
+			gw.reader.
+				On("GetTrace", matchContext, tc.expectedQuery).
+				Return(&model.Trace{}, nil).Once()
+
+			q := url.Values{}
+			for k, v := range tc.params {
+				q.Set(k, v)
+			}
+			testUrl := testUri
+			if len(tc.params) > 0 {
+				testUrl += "?" + q.Encode()
+			}
+
+			r, err := http.NewRequest(http.MethodGet, testUrl, nil)
+			require.NoError(t, err)
+			w := httptest.NewRecorder()
+			gw.router.ServeHTTP(w, r)
+			gw.reader.AssertCalled(t, "GetTrace", matchContext, tc.expectedQuery)
+		})
+	}
+}
+
+func TestHTTPGatewayGetTraceMalformedInputErrors(t *testing.T) {
+	testCases := []struct {
+		name          string
+		requestUrl    string
+		expectedError string
+	}{
+		{
+			name:          "TestGetTrace",
+			requestUrl:    "/api/v3/traces/xyz",
+			expectedError: "malformed parameter trace_id",
+		},
+		{
+			name:          "TestGetTraceWithInvalidStartTime",
+			requestUrl:    "/api/v3/traces/123?start_time=abc",
+			expectedError: "malformed parameter start_time",
+		},
+		{
+			name:          "TestGetTraceWithInvalidEndTime",
+			requestUrl:    "/api/v3/traces/123?end_time=xyz",
+			expectedError: "malformed parameter end_time",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			gw := setupHTTPGatewayNoServer(t, "")
+			gw.reader.
+				On("GetTrace", matchContext, matchGetTraceParameters).
+				Return(&model.Trace{}, nil).Once()
+
+			r, err := http.NewRequest(http.MethodGet, tc.requestUrl, nil)
+			require.NoError(t, err)
+			w := httptest.NewRecorder()
+			gw.router.ServeHTTP(w, r)
+			assert.Contains(t, w.Body.String(), tc.expectedError)
+		})
+	}
+}
+
+func TestHTTPGatewayGetTraceInternalErrors(t *testing.T) {
+	gw := setupHTTPGatewayNoServer(t, "")
 	gw.reader.
 		On("GetTrace", matchContext, matchGetTraceParameters).
-		Return(nil, errors.New(simErr)).Once()
+		Return(nil, assert.AnError).Once()
 
-	r, err = http.NewRequest(http.MethodGet, "/api/v3/traces/123", nil)
+	r, err := http.NewRequest(http.MethodGet, "/api/v3/traces/123", nil)
 	require.NoError(t, err)
-	w = httptest.NewRecorder()
+	w := httptest.NewRecorder()
 	gw.router.ServeHTTP(w, r)
-	assert.Contains(t, w.Body.String(), simErr)
+	assert.Contains(t, w.Body.String(), assert.AnError.Error())
 }
 
 func mockFindQueries() (url.Values, *spanstore.TraceQueryParameters) {

cmd/query/app/querysvc/adjuster/spanlinks.go

@@ -5,6 +5,12 @@ package adjuster
 
 import (
 	"go.opentelemetry.io/collector/pdata/ptrace"
+
+	"github.com/jaegertracing/jaeger/internal/jptrace"
+)
+
+const (
+	invalidSpanLinkWarning = "Invalid span link removed"
 )
 
 var _ Adjuster = (*LinksAdjuster)(nil)
@@ -42,6 +48,8 @@ func (la LinksAdjuster) adjust(span ptrace.Span) {
 		if la.valid(link) {
 			newLink := validLinks.AppendEmpty()
 			link.CopyTo(newLink)
+		} else {
+			jptrace.AddWarning(span, invalidSpanLinkWarning)
 		}
 	}
 	validLinks.CopyTo(span.Links())

cmd/query/app/querysvc/adjuster/spanlinks_test.go

@@ -10,6 +10,8 @@ import (
 	"github.com/stretchr/testify/require"
 	"go.opentelemetry.io/collector/pdata/pcommon"
 	"go.opentelemetry.io/collector/pdata/ptrace"
+
+	"github.com/jaegertracing/jaeger/internal/jptrace"
 )
 
 func TestLinksAdjuster(t *testing.T) {
@@ -21,19 +23,32 @@ func TestLinksAdjuster(t *testing.T) {
 	scopeSpans.Spans().AppendEmpty()
 
 	// span with empty traceID link
-	spanA := scopeSpans.Spans().AppendEmpty()
-	spanA.Links().AppendEmpty().SetTraceID(pcommon.NewTraceIDEmpty())
+	spanB := scopeSpans.Spans().AppendEmpty()
+	spanB.Links().AppendEmpty().SetTraceID(pcommon.NewTraceIDEmpty())
 
 	// span with 2 non-empty traceID links and 1 empty (or zero) traceID link
-	spanB := scopeSpans.Spans().AppendEmpty()
-	spanB.Links().AppendEmpty().SetTraceID(pcommon.TraceID([]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}))
-	spanB.Links().AppendEmpty().SetTraceID(pcommon.TraceID([]byte{0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0}))
-	spanB.Links().AppendEmpty().SetTraceID(pcommon.TraceID([]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}))
+	spanC := scopeSpans.Spans().AppendEmpty()
+	spanC.Links().AppendEmpty().SetTraceID(pcommon.TraceID([]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}))
+	spanC.Links().AppendEmpty().SetTraceID(pcommon.TraceID([]byte{0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0}))
+	spanC.Links().AppendEmpty().SetTraceID(pcommon.TraceID([]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}))
 
 	err := SpanLinks().Adjust(traces)
 	spans := traces.ResourceSpans().At(0).ScopeSpans().At(0).Spans()
 	require.NoError(t, err)
-	assert.Equal(t, 0, spans.At(0).Links().Len())
-	assert.Equal(t, 0, spans.At(1).Links().Len())
-	assert.Equal(t, 2, spans.At(2).Links().Len())
+
+	gotSpansA := spans.At(0)
+	assert.Equal(t, 0, gotSpansA.Links().Len())
+	assert.Empty(t, jptrace.GetWarnings(gotSpansA))
+
+	gotSpansB := spans.At(1)
+	assert.Equal(t, 0, gotSpansB.Links().Len())
+	spanBWarnings := jptrace.GetWarnings(gotSpansB)
+	assert.Len(t, spanBWarnings, 1)
+	assert.Equal(t, "Invalid span link removed", spanBWarnings[0])
+
+	gotSpansC := spans.At(2)
+	assert.Equal(t, 2, gotSpansC.Links().Len())
+	spanCWarnings := jptrace.GetWarnings(gotSpansC)
+	assert.Len(t, spanCWarnings, 1)
+	assert.Equal(t, "Invalid span link removed", spanCWarnings[0])
 }