Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[api_v3][query] Change api_v3 http handler to use v2 query service #6459

Merged
merged 12 commits into from
Jan 3, 2025
Merged

[api_v3][query] Change api_v3 http handler to use v2 query service #6459

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
bab92e8
Remove Helper Function Not Required
mahadzaryab1 Jan 1, 2025
976a0e5
Change api_v3 Handler To Use V2 Query Service
mahadzaryab1 Jan 1, 2025
cc103c9
Remove Double Translation
mahadzaryab1 Jan 1, 2025
19f8120
Merge branch 'main' into api-v3-http-qs
mahadzaryab1 Jan 2, 2025
e9bcd49
Fix Get Trace Success Unit Test
mahadzaryab1 Jan 2, 2025
adaf93d
Merge branch 'main' into api-v3-http-qs
mahadzaryab1 Jan 3, 2025
6595cc3
Remove Aggregation And Return Not Found Error
mahadzaryab1 Jan 3, 2025
48b851c
Fix Test Expectations
mahadzaryab1 Jan 3, 2025
39c435b
Fix Tests And Regenerate Snapshots
mahadzaryab1 Jan 3, 2025
d5c5a58
Move Common Logic To Helper
mahadzaryab1 Jan 3, 2025
a7eb115
Add TODO Comment
mahadzaryab1 Jan 3, 2025
a56567b
Fix Missing Code Coverage
mahadzaryab1 Jan 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 23 additions & 23 deletions cmd/query/app/apiv3/http_gateway.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,16 @@ import (
"github.com/gogo/protobuf/jsonpb"
"github.com/gogo/protobuf/proto"
"github.com/gorilla/mux"
"go.opentelemetry.io/collector/pdata/ptrace"
"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
"go.opentelemetry.io/otel/trace"
"go.uber.org/zap"

"github.com/jaegertracing/jaeger/cmd/query/app/querysvc"
"github.com/jaegertracing/jaeger/cmd/query/app/querysvc/v2/querysvc"
"github.com/jaegertracing/jaeger/internal/proto/api_v3"
"github.com/jaegertracing/jaeger/model"
"github.com/jaegertracing/jaeger/storage/spanstore"
"github.com/jaegertracing/jaeger/storage_v2/tracestore"
"github.com/jaegertracing/jaeger/storage_v2/v1adapter"
)

const (
Expand Down Expand Up @@ -109,15 +110,6 @@ func (h *HTTPGateway) tryParamError(w http.ResponseWriter, err error, paramName
}

func (h *HTTPGateway) returnSpans(spans []*model.Span, w http.ResponseWriter) {
// modelToOTLP does not easily return an error, so allow mocking it
h.returnSpansTestable(spans, w, modelToOTLP)
}

func (h *HTTPGateway) returnSpansTestable(
spans []*model.Span,
w http.ResponseWriter,
modelToOTLP func(_ []*model.Span) ptrace.Traces,
) {
td := modelToOTLP(spans)
tracesData := api_v3.TracesData(td)
response := &api_v3.GRPCGatewayWrapper{
Expand All @@ -137,9 +129,11 @@ func (h *HTTPGateway) getTrace(w http.ResponseWriter, r *http.Request) {
if h.tryParamError(w, err, paramTraceID) {
return
}
request := querysvc.GetTraceParameters{
GetTraceParameters: spanstore.GetTraceParameters{
TraceID: traceID,
request := querysvc.GetTraceParams{
TraceIDs: []tracestore.GetTraceParams{
{
TraceID: traceID.ToOTELTraceID(),
},
},
}
http_query := r.URL.Query()
Expand All @@ -149,15 +143,15 @@ func (h *HTTPGateway) getTrace(w http.ResponseWriter, r *http.Request) {
if h.tryParamError(w, err, paramStartTime) {
return
}
request.StartTime = timeParsed.UTC()
request.TraceIDs[0].Start = 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()
request.TraceIDs[0].End = timeParsed.UTC()
}
if r := http_query.Get(paramRawTraces); r != "" {
rawTraces, err := strconv.ParseBool(r)
Expand All @@ -166,11 +160,16 @@ func (h *HTTPGateway) getTrace(w http.ResponseWriter, r *http.Request) {
}
request.RawTraces = rawTraces
}
trc, err := h.QueryService.GetTrace(r.Context(), request)
getTracesIter := h.QueryService.GetTraces(r.Context(), request)
trc, err := v1adapter.V1TracesFromSeq2(getTracesIter)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it doesn't make sense to convert to v1 model only to convert it back to OTLP in L113. The benefit of v2 storage here is that we can avoid any unnecessary transforms

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yurishkuro yep - will fix. I was just trying to wire up the new query service first.

if h.tryHandleError(w, err, http.StatusInternalServerError) {
return
}
h.returnSpans(trc.Spans, w)
if len(trc) == 0 {
// TODO: should we return 404 if trace not found?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I think this is the contract of v3 API (which is unfortunate since it doesn't match the Storage v2 API contract, but we'll keep it for backwards compatibility).

return
}
h.returnSpans(trc[0].Spans, w)
}

func (h *HTTPGateway) findTraces(w http.ResponseWriter, r *http.Request) {
Expand All @@ -179,7 +178,8 @@ func (h *HTTPGateway) findTraces(w http.ResponseWriter, r *http.Request) {
return
}

traces, err := h.QueryService.FindTraces(r.Context(), queryParams)
findTracesIter := h.QueryService.FindTraces(r.Context(), *queryParams)
traces, err := v1adapter.V1TracesFromSeq2(findTracesIter)
// TODO how do we distinguish internal error from bad parameters for FindTrace?
if h.tryHandleError(w, err, http.StatusInternalServerError) {
return
Expand All @@ -191,9 +191,9 @@ func (h *HTTPGateway) findTraces(w http.ResponseWriter, r *http.Request) {
h.returnSpans(spans, w)
}

func (h *HTTPGateway) parseFindTracesQuery(q url.Values, w http.ResponseWriter) (*querysvc.TraceQueryParameters, bool) {
queryParams := &querysvc.TraceQueryParameters{
TraceQueryParameters: spanstore.TraceQueryParameters{
func (h *HTTPGateway) parseFindTracesQuery(q url.Values, w http.ResponseWriter) (*querysvc.TraceQueryParams, bool) {
queryParams := &querysvc.TraceQueryParams{
TraceQueryParams: tracestore.TraceQueryParams{
ServiceName: q.Get(paramServiceName),
OperationName: q.Get(paramOperationName),
Tags: nil, // most curiously not supported by grpc-gateway
Expand Down Expand Up @@ -262,7 +262,7 @@ func (h *HTTPGateway) getServices(w http.ResponseWriter, r *http.Request) {

func (h *HTTPGateway) getOperations(w http.ResponseWriter, r *http.Request) {
query := r.URL.Query()
queryParams := spanstore.OperationQueryParameters{
queryParams := tracestore.OperationQueryParams{
ServiceName: query.Get("service"),
SpanKind: query.Get("span_kind"),
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/query/app/apiv3/http_gateway_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
"github.com/stretchr/testify/require"
"go.uber.org/zap"

"github.com/jaegertracing/jaeger/cmd/query/app/querysvc"
"github.com/jaegertracing/jaeger/cmd/query/app/querysvc/v2/querysvc"
"github.com/jaegertracing/jaeger/model"
"github.com/jaegertracing/jaeger/pkg/jtracer"
"github.com/jaegertracing/jaeger/pkg/testutils"
Expand Down
14 changes: 8 additions & 6 deletions cmd/query/app/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (

"github.com/jaegertracing/jaeger/cmd/query/app/apiv3"
"github.com/jaegertracing/jaeger/cmd/query/app/querysvc"
querysvcv2 "github.com/jaegertracing/jaeger/cmd/query/app/querysvc/v2/querysvc"
v2querysvc "github.com/jaegertracing/jaeger/cmd/query/app/querysvc/v2/querysvc"
"github.com/jaegertracing/jaeger/internal/proto/api_v3"
"github.com/jaegertracing/jaeger/pkg/bearertoken"
"github.com/jaegertracing/jaeger/pkg/netutils"
Expand Down Expand Up @@ -59,7 +59,7 @@ type Server struct {
func NewServer(
ctx context.Context,
querySvc *querysvc.QueryService,
v2QuerySvc *querysvcv2.QueryService,
v2QuerySvc *v2querysvc.QueryService,
metricsQuerySvc querysvc.MetricsQueryService,
options *QueryOptions,
tm *tenancy.Manager,
Expand All @@ -84,7 +84,7 @@ func NewServer(
return nil, err
}
registerGRPCHandlers(grpcServer, querySvc, v2QuerySvc, metricsQuerySvc, telset)
httpServer, err := createHTTPServer(ctx, querySvc, metricsQuerySvc, options, tm, telset)
httpServer, err := createHTTPServer(ctx, querySvc, v2QuerySvc, metricsQuerySvc, options, tm, telset)
if err != nil {
return nil, err
}
Expand All @@ -102,7 +102,7 @@ func NewServer(
func registerGRPCHandlers(
server *grpc.Server,
querySvc *querysvc.QueryService,
v2QuerySvc *querysvcv2.QueryService,
v2QuerySvc *v2querysvc.QueryService,
metricsQuerySvc querysvc.MetricsQueryService,
telset telemetry.Settings,
) {
Expand Down Expand Up @@ -167,6 +167,7 @@ var _ io.Closer = (*httpServer)(nil)

func initRouter(
querySvc *querysvc.QueryService,
v2QuerySvc *v2querysvc.QueryService,
metricsQuerySvc querysvc.MetricsQueryService,
queryOpts *QueryOptions,
tenancyMgr *tenancy.Manager,
Expand All @@ -187,7 +188,7 @@ func initRouter(
}

(&apiv3.HTTPGateway{
QueryService: querySvc,
QueryService: v2QuerySvc,
Logger: telset.Logger,
Tracer: telset.TracerProvider,
}).RegisterRoutes(r)
Expand All @@ -209,12 +210,13 @@ func initRouter(
func createHTTPServer(
ctx context.Context,
querySvc *querysvc.QueryService,
v2QuerySvc *v2querysvc.QueryService,
metricsQuerySvc querysvc.MetricsQueryService,
queryOpts *QueryOptions,
tm *tenancy.Manager,
telset telemetry.Settings,
) (*httpServer, error) {
handler, staticHandlerCloser := initRouter(querySvc, metricsQuerySvc, queryOpts, tm, telset)
handler, staticHandlerCloser := initRouter(querySvc, v2QuerySvc, metricsQuerySvc, queryOpts, tm, telset)
handler = recoveryhandler.NewRecoveryHandler(telset.Logger, true)(handler)
hs, err := queryOpts.HTTP.ToServer(
ctx,
Expand Down
Loading