From ac9259d6a4de3f43ca164e29bc4f06bb01239b6b Mon Sep 17 00:00:00 2001 From: ismail simsek Date: Mon, 10 Nov 2025 17:47:29 +0100 Subject: [PATCH] Chore: Update pyroscope error sources logging (#113175) * remove loggers * more downstream errors * more downstream errors 2 * uber nit --- .../grafana-pyroscope-datasource/instance.go | 61 ++++++------------- .../pyroscopeClient.go | 9 +-- .../grafana-pyroscope-datasource/service.go | 36 +++++------ 3 files changed, 37 insertions(+), 69 deletions(-) diff --git a/pkg/tsdb/grafana-pyroscope-datasource/instance.go b/pkg/tsdb/grafana-pyroscope-datasource/instance.go index e2fe25c94ef..244481aacbd 100644 --- a/pkg/tsdb/grafana-pyroscope-datasource/instance.go +++ b/pkg/tsdb/grafana-pyroscope-datasource/instance.go @@ -3,7 +3,6 @@ package pyroscope import ( "context" "encoding/json" - "fmt" "net/http" "net/url" "slices" @@ -46,16 +45,13 @@ type PyroscopeDatasource struct { // NewPyroscopeDatasource creates a new datasource instance. func NewPyroscopeDatasource(ctx context.Context, httpClientProvider httpclient.Provider, settings backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) { - ctxLogger := logger.FromContext(ctx) opt, err := settings.HTTPClientOptions(ctx) if err != nil { - ctxLogger.Error("Failed to get HTTP client options", "error", err, "function", logEntrypoint()) - return nil, err + return nil, backend.DownstreamErrorf("failed to get HTTP client options: %w. function: %s", err, logEntrypoint()) } httpClient, err := httpClientProvider.New(opt) if err != nil { - ctxLogger.Error("Failed to create HTTP client", "error", err, "function", logEntrypoint()) - return nil, err + return nil, backend.DownstreamErrorf("failed to create HTTP client: %w. function: %s", err, logEntrypoint()) } return &PyroscopeDatasource{ @@ -88,12 +84,9 @@ func (d *PyroscopeDatasource) CallResource(ctx context.Context, req *backend.Cal } func (d *PyroscopeDatasource) profileTypes(ctx context.Context, req *backend.CallResourceRequest, sender backend.CallResourceResponseSender) error { - ctxLogger := logger.FromContext(ctx) - u, err := url.Parse(req.URL) if err != nil { - ctxLogger.Error("Failed to parse URL", "error", err, "function", logEntrypoint()) - return backend.DownstreamErrorf("URL could not be parsed: %w", err) + return backend.DownstreamErrorf("URL could not be parsed: %w. function: %s", err, logEntrypoint()) } query := u.Query() @@ -101,14 +94,12 @@ func (d *PyroscopeDatasource) profileTypes(ctx context.Context, req *backend.Cal if query.Has("start") && query.Has("end") { start, err = strconv.ParseInt(query.Get("start"), 10, 64) if err != nil { - ctxLogger.Error("Failed to parse start as int", "error", err, "function", logEntrypoint()) - return backend.DownstreamError(fmt.Errorf("failed to parse start as int: %w", err)) + return backend.DownstreamErrorf("failed to parse start as int: %w. function: %s", err, logEntrypoint()) } end, err = strconv.ParseInt(query.Get("end"), 10, 64) if err != nil { - ctxLogger.Error("Failed to parse end as int", "error", err, "function", logEntrypoint()) - return backend.DownstreamError(fmt.Errorf("failed to parse end as int: %w", err)) + return backend.DownstreamErrorf("failed to parse end as int: %w. function: %s", err, logEntrypoint()) } } else { // Make sure to pass a valid time range to the client as v2 will not work without it. @@ -118,29 +109,23 @@ func (d *PyroscopeDatasource) profileTypes(ctx context.Context, req *backend.Cal types, err := d.client.ProfileTypes(ctx, start, end) if err != nil { - ctxLogger.Error("Received error from client", "error", err, "function", logEntrypoint()) - return err + return backend.DownstreamErrorf("received error from client: %w. function: %s", err, logEntrypoint()) } bodyData, err := json.Marshal(types) if err != nil { - ctxLogger.Error("Failed to marshal response", "error", err, "function", logEntrypoint()) - return err + return backend.DownstreamErrorf("failed to marshal response: %w. function: %s", err, logEntrypoint()) } err = sender.Send(&backend.CallResourceResponse{Body: bodyData, Status: 200}) if err != nil { - ctxLogger.Error("Failed to send response", "error", err, "function", logEntrypoint()) - return err + return backend.DownstreamErrorf("failed to send response: %w. function: %s", err, logEntrypoint()) } return nil } func (d *PyroscopeDatasource) labelNames(ctx context.Context, req *backend.CallResourceRequest, sender backend.CallResourceResponseSender) error { - ctxLogger := logger.FromContext(ctx) - u, err := url.Parse(req.URL) if err != nil { - ctxLogger.Error("Failed to parse URL", "error", err, "function", logEntrypoint()) - return backend.DownstreamError(fmt.Errorf("URL could not be parsed: %w", err)) + return backend.DownstreamErrorf("URL could not be parsed: %w. function: %s", err, logEntrypoint()) } query := u.Query() @@ -149,14 +134,12 @@ func (d *PyroscopeDatasource) labelNames(ctx context.Context, req *backend.CallR labelSelector := query.Get("query") matchers, err := parser.ParseMetricSelector(labelSelector) if err != nil { - ctxLogger.Error("Could not parse label selector", "error", err, "function", logEntrypoint()) - return backend.DownstreamError(fmt.Errorf("failed parsing label selector: %v", err)) + return backend.DownstreamErrorf("failed parsing label selector: %w. function: %s", err, logEntrypoint()) } labelNames, err := d.client.LabelNames(ctx, labelSelector, start, end) if err != nil { - ctxLogger.Error("Received error from client", "error", err, "function", logEntrypoint()) - return backend.DownstreamError(fmt.Errorf("error calling LabelNames: %v", err)) + return backend.DownstreamErrorf("error calling LabelNames: %w. function: %s", err, logEntrypoint()) } finalLabels := make([]string, 0) @@ -171,13 +154,11 @@ func (d *PyroscopeDatasource) labelNames(ctx context.Context, req *backend.CallR jsonResponse, err := json.Marshal(finalLabels) if err != nil { - ctxLogger.Error("Failed to marshal response", "error", err, "function", logEntrypoint()) - return err + return backend.DownstreamErrorf("failed to marshal response: %w. function: %s", err, logEntrypoint()) } err = sender.Send(&backend.CallResourceResponse{Body: jsonResponse, Status: 200}) if err != nil { - ctxLogger.Error("Failed to send response", "error", err, "function", logEntrypoint()) - return err + return backend.DownstreamErrorf("failed to send response: %w. function: %s", err, logEntrypoint()) } return nil } @@ -190,11 +171,9 @@ type LabelValuesPayload struct { } func (d *PyroscopeDatasource) labelValues(ctx context.Context, req *backend.CallResourceRequest, sender backend.CallResourceResponseSender) error { - ctxLogger := logger.FromContext(ctx) u, err := url.Parse(req.URL) if err != nil { - ctxLogger.Error("Failed to parse URL", "error", err, "function", logEntrypoint()) - return backend.DownstreamError(fmt.Errorf("URL could not be parsed: %w", err)) + return backend.DownstreamErrorf("URL could not be parsed: %w. function: %s", err, logEntrypoint()) } query := u.Query() @@ -204,20 +183,17 @@ func (d *PyroscopeDatasource) labelValues(ctx context.Context, req *backend.Call res, err := d.client.LabelValues(ctx, label, query.Get("query"), start, end) if err != nil { - ctxLogger.Error("Received error from client", "error", err, "function", logEntrypoint()) - return backend.DownstreamError(fmt.Errorf("error calling LabelValues: %v", err)) + return backend.DownstreamErrorf("error calling LabelValues: %w. function: %s", err, logEntrypoint()) } data, err := json.Marshal(res) if err != nil { - ctxLogger.Error("Failed to marshal response", "error", err, "function", logEntrypoint()) - return backend.DownstreamErrorf("failed to marshall response: %w", err) + return backend.DownstreamErrorf("failed to marshall response: %w. function: %s", err, logEntrypoint()) } err = sender.Send(&backend.CallResourceResponse{Body: data, Status: 200}) if err != nil { - ctxLogger.Error("Failed to send response", "error", err, "function", logEntrypoint()) - return err + return backend.DownstreamErrorf("failed to send response: %w. function: %s", err, logEntrypoint()) } return nil @@ -227,13 +203,10 @@ func (d *PyroscopeDatasource) labelValues(ctx context.Context, req *backend.Call // for all known profile types, including their aggregation type (cumulative/instant), // units, descriptions, and grouping information. func (d *PyroscopeDatasource) profileMetadata(ctx context.Context, _ *backend.CallResourceRequest, sender backend.CallResourceResponseSender) error { - ctxLogger := logger.FromContext(ctx) - registry := GetProfileMetadataRegistry() jsonData, err := json.Marshal(registry.profiles) if err != nil { - ctxLogger.Error("Failed to marshal profile metadata", "error", err, "function", logEntrypoint()) return sender.Send(&backend.CallResourceResponse{ Status: 500, Body: []byte(`{"error": "Failed to marshal profile metadata"}`), diff --git a/pkg/tsdb/grafana-pyroscope-datasource/pyroscopeClient.go b/pkg/tsdb/grafana-pyroscope-datasource/pyroscopeClient.go index da5d9309130..aa6af21d6c3 100644 --- a/pkg/tsdb/grafana-pyroscope-datasource/pyroscopeClient.go +++ b/pkg/tsdb/grafana-pyroscope-datasource/pyroscopeClient.go @@ -80,7 +80,6 @@ func (c *PyroscopeClient) ProfileTypes(ctx context.Context, start int64, end int End: end, })) if err != nil { - logger.Error("Received error from client", "error", err, "function", logEntrypoint()) span.RecordError(err) span.SetStatus(codes.Error, err.Error()) return nil, backend.DownstreamError(fmt.Errorf("received error from client while getting profile types: %w", err)) @@ -115,10 +114,9 @@ func (c *PyroscopeClient) GetSeries(ctx context.Context, profileTypeID string, l resp, err := c.connectClient.SelectSeries(ctx, req) if err != nil { - logger.Error("Received error from client", "error", err, "function", logEntrypoint()) span.RecordError(err) span.SetStatus(codes.Error, err.Error()) - return nil, backend.DownstreamError(fmt.Errorf("received error from client while getting series: %w", err)) + return nil, backend.DownstreamErrorf("received error from client while getting series: %w", err) } series := make([]*Series, len(resp.Msg.Series)) @@ -171,7 +169,6 @@ func (c *PyroscopeClient) GetProfile(ctx context.Context, profileTypeID, labelSe resp, err := c.connectClient.SelectMergeStacktraces(ctx, req) if err != nil { - logger.Error("Received error from client", "error", err, "function", logEntrypoint()) span.RecordError(err) span.SetStatus(codes.Error, err.Error()) return nil, backend.DownstreamError(fmt.Errorf("received error from client while getting profile: %w", err)) @@ -207,7 +204,7 @@ func (c *PyroscopeClient) GetSpanProfile(ctx context.Context, profileTypeID, lab } if resp.Msg.Flamegraph == nil { - // Not an error, can happen when querying data oout of range. + // Not an error, can happen when querying data out of range. return nil, nil } @@ -254,7 +251,6 @@ func (c *PyroscopeClient) LabelNames(ctx context.Context, labelSelector string, End: end, })) if err != nil { - logger.Error("Received error from client", "error", err, "function", logEntrypoint()) span.RecordError(err) span.SetStatus(codes.Error, err.Error()) return nil, backend.DownstreamError(fmt.Errorf("error sending LabelNames request %v", err)) @@ -284,7 +280,6 @@ func (c *PyroscopeClient) LabelValues(ctx context.Context, label string, labelSe End: end, })) if err != nil { - logger.Error("Received error from client", "error", err, "function", logEntrypoint()) span.RecordError(err) span.SetStatus(codes.Error, err.Error()) return nil, backend.DownstreamError(fmt.Errorf("received error from client while getting label values: %w", err)) diff --git a/pkg/tsdb/grafana-pyroscope-datasource/service.go b/pkg/tsdb/grafana-pyroscope-datasource/service.go index bad52d48984..04a4c5240fc 100644 --- a/pkg/tsdb/grafana-pyroscope-datasource/service.go +++ b/pkg/tsdb/grafana-pyroscope-datasource/service.go @@ -87,10 +87,10 @@ func (s *Service) QueryData(ctx context.Context, req *backend.QueryDataRequest) response, err := i.QueryData(ctx, req) if err != nil { - ctxLogger.Error("Received error from Pyroscope", "error", err, "function", logEntrypoint()) - } else { - ctxLogger.Debug("All queries processed", "function", logEntrypoint()) + return nil, backend.DownstreamErrorf("received error from Pyroscope while querying data: %s", err) } + + ctxLogger.Debug("All queries processed", "function", logEntrypoint()) return response, err } @@ -105,10 +105,10 @@ func (s *Service) CallResource(ctx context.Context, req *backend.CallResourceReq err = i.CallResource(ctx, req, sender) if err != nil { - loggerWithContext.Error("Received error from Pyroscope", "error", err, "function", logEntrypoint()) - } else { - loggerWithContext.Debug("Health check succeeded", "function", logEntrypoint()) + return backend.DownstreamErrorf("received error from Pyroscope while calling resource: %w", err) } + + loggerWithContext.Debug("Health check succeeded", "function", logEntrypoint()) return err } @@ -123,10 +123,10 @@ func (s *Service) CheckHealth(ctx context.Context, req *backend.CheckHealthReque response, err := i.CheckHealth(ctx, req) if err != nil { - loggerWithContext.Error("Received error from Pyroscope", "error", err, "function", logEntrypoint()) - } else { - loggerWithContext.Debug("Health check succeeded", "function", logEntrypoint()) + return nil, backend.DownstreamErrorf("received error from Pyroscope while health checking: %w", err) } + + loggerWithContext.Debug("Health check succeeded", "function", logEntrypoint()) return response, err } @@ -141,10 +141,10 @@ func (s *Service) SubscribeStream(ctx context.Context, req *backend.SubscribeStr response, err := i.SubscribeStream(ctx, req) if err != nil { - loggerWithContext.Error("Received error from Pyroscope", "error", err, "function", logEntrypoint()) - } else { - loggerWithContext.Debug("Stream subscribed", "function", logEntrypoint()) + return nil, backend.DownstreamErrorf("received error from Pyroscope while subscribing the stream: %w", err) } + + loggerWithContext.Debug("Stream subscribed", "function", logEntrypoint()) return response, err } @@ -159,10 +159,10 @@ func (s *Service) RunStream(ctx context.Context, req *backend.RunStreamRequest, err = i.RunStream(ctx, req, sender) if err != nil { - loggerWithContext.Error("Received error from Pyroscope", "error", err, "function", logEntrypoint()) - } else { - loggerWithContext.Debug("Stream run", "function", logEntrypoint()) + return backend.DownstreamErrorf("received error from Pyroscope while trying to run the stream: %w", err) } + + loggerWithContext.Debug("Stream run", "function", logEntrypoint()) return err } @@ -178,9 +178,9 @@ func (s *Service) PublishStream(ctx context.Context, req *backend.PublishStreamR response, err := i.PublishStream(ctx, req) if err != nil { - loggerWithContext.Error("Received error from Pyroscope", "error", err, "function", logEntrypoint()) - } else { - loggerWithContext.Debug("Stream published", "function", logEntrypoint()) + return nil, backend.DownstreamErrorf("received error from Pyroscope while publishing the stream: %w", err) } + + loggerWithContext.Debug("Stream published", "function", logEntrypoint()) return response, err }