Skip to content

Conversation

@Divyansh200102
Copy link

@Divyansh200102 Divyansh200102 commented Nov 3, 2025

Which problem is this PR solving?

Description of the changes

This PR adds support for filtering by query.attributes in the /api/v3/traces HTTP endpoint. Previously, the query.attributes parameter was ignored by the handler.

  • cmd/query/app/apiv3/http_gateway.go:
    • Updated parseFindTracesQuery to read the query.attributes parameter.
    • Added logic to unmarshal the attributes JSON string and populate the TraceQueryParams struct.

How was this change tested?

  • Added new unit tests in cmd/query/app/apiv3/http_gateway_test.go:
    • TestHTTPGatewayFindTracesWithAttributes: Verifies that the query.attributes are correctly parsed and passed to the query service.
    • Added a new test case to TestHTTPGatewayFindTracesErrors to check for malformed query.attributes JSON.
  • All tests were run successfully.

Screenshot

image

Checklist

cc @yurishkuro

@Divyansh200102 Divyansh200102 requested a review from a team as a code owner November 3, 2025 19:24
@dosubot dosubot bot added the enhancement label Nov 3, 2025
Comment on lines 220 to 226
if err := json.Unmarshal([]byte(attrStr), &attrMap); err != nil {
// Pass the underlying JSON parsing error
err := fmt.Errorf("cannot parse attributes JSON: %w", err)
if h.tryParamError(w, err, paramAttributes) {
return nil, true
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a bug in the error handling logic. When JSON unmarshal fails and h.tryParamError() returns true (indicating the error was handled), the function continues execution with an empty attributes map instead of returning. This could lead to unexpected behavior.

The code should be modified to return immediately after handling the error:

if err := json.Unmarshal([]byte(attrStr), &attrMap); err != nil {
    // Pass the underlying JSON parsing error
    err := fmt.Errorf("cannot parse attributes JSON: %w", err)
    if h.tryParamError(w, err, paramAttributes) {
        return nil, true
    }
}

Without a proper return statement, the function will continue processing with potentially invalid input data.

Suggested change
if err := json.Unmarshal([]byte(attrStr), &attrMap); err != nil {
// Pass the underlying JSON parsing error
err := fmt.Errorf("cannot parse attributes JSON: %w", err)
if h.tryParamError(w, err, paramAttributes) {
return nil, true
}
}
if err := json.Unmarshal([]byte(attrStr), &attrMap); err != nil {
// Pass the underlying JSON parsing error
err := fmt.Errorf("cannot parse attributes JSON: %w", err)
if h.tryParamError(w, err, paramAttributes) {
return nil, true
}
return nil, true
}

Spotted by Graphite Agent

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

@Divyansh200102
Copy link
Author

@albertteoh can I get a review on this PR?

@yurishkuro
Copy link
Member

there have to be IDL changes first that document changes to the schema

@Divyansh200102
Copy link
Author

there have to be IDL changes first that document changes to the schema

@yurishkuro I checked the proto file and the attributes field already exists in TraceQueryParameters (query_service.proto line 45). My PR is implementing the parsing logic that was missing in the HTTP gateway.
Do I still need an IDL PR, or can we proceed since the schema already documents this field?

@yurishkuro
Copy link
Member

ok, I see the IDL has the attributes, but where is it defined that the new param is supposed to be url-encoded JSON?

@Divyansh200102
Copy link
Author

Divyansh200102 commented Nov 11, 2025

ok, I see the IDL has the attributes, but where is it defined that the new param is supposed to be url-encoded JSON?

You're right - the HTTP format isn't explicitly documented.I have made the PR and fixed a small error in CONTRIBUTING.md as well please review it.
jaegertracing/jaeger-idl#176

@yurishkuro
Copy link
Member

does this match how UI sends tags filter?

@Divyansh200102
Copy link
Author

Divyansh200102 commented Nov 11, 2025

does this match how UI sends tags filter?

Yes, this matches how the UI sends the tags filter.The UI sends: tags=%7B%22http.status_code%22%3A%22200%22%2C%22error%22%3A%22true%22%7D for tag http.status_code=200 error=true
Which decodes to: {"http.status_code":"200","error":"true"}

if err := json.Unmarshal([]byte(attrStr), &attrMap); err != nil {
// Pass the underlying JSON parsing error
err := fmt.Errorf("cannot parse attributes JSON: %w", err)
if h.tryParamError(w, err, paramAttributes) {
Copy link
Member

Choose a reason for hiding this comment

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

tryParamError is used when you're not sure if err is not nil. Here you do, so should use tryHandleError

}

func (h *HTTPGateway) parseFindTracesQuery(q url.Values, w http.ResponseWriter) (*querysvc.TraceQueryParams, bool) {
// Parse attributes first
Copy link
Member

Choose a reason for hiding this comment

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

Please move to a helper function

@codecov
Copy link

codecov bot commented Nov 11, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 40.32%. Comparing base (39ef71e) to head (c4766f2).
⚠️ Report is 3 commits behind head on main.

❌ Your project check has failed because the head coverage (40.32%) is below the target coverage (95.00%). You can increase the head coverage or adjust the target coverage.

❗ There is a different number of reports uploaded between BASE (39ef71e) and HEAD (c4766f2). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (39ef71e) HEAD (c4766f2)
unittests 1 0
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #7631       +/-   ##
===========================================
- Coverage   96.55%   40.32%   -56.24%     
===========================================
  Files         384      212      -172     
  Lines       19473    11550     -7923     
===========================================
- Hits        18803     4658    -14145     
- Misses        486     6452     +5966     
- Partials      184      440      +256     
Flag Coverage Δ
badger_v1 8.82% <ø> (ø)
badger_v2 1.72% <ø> (ø)
cassandra-4.x-v1-manual 12.55% <ø> (ø)
cassandra-4.x-v2-auto 1.72% <ø> (ø)
cassandra-4.x-v2-manual 1.72% <ø> (ø)
cassandra-5.x-v1-manual 12.55% <ø> (ø)
cassandra-5.x-v2-auto 1.72% <ø> (ø)
cassandra-5.x-v2-manual 1.72% <ø> (ø)
clickhouse 1.65% <ø> (ø)
elasticsearch-6.x-v1 16.75% <ø> (ø)
elasticsearch-7.x-v1 16.78% <ø> (ø)
elasticsearch-8.x-v1 16.92% <ø> (ø)
elasticsearch-8.x-v2 1.72% <ø> (ø)
elasticsearch-9.x-v2 1.72% <ø> (ø)
grpc_v1 10.75% <ø> (ø)
grpc_v2 1.72% <ø> (ø)
kafka-3.x-v1 10.25% <ø> (ø)
kafka-3.x-v2 1.72% <ø> (ø)
memory_v2 1.72% <ø> (ø)
opensearch-1.x-v1 16.82% <ø> (ø)
opensearch-2.x-v1 16.82% <ø> (ø)
opensearch-2.x-v2 1.72% <ø> (ø)
opensearch-3.x-v2 1.72% <ø> (ø)
query 1.72% <ø> (ø)
tailsampling-processor 0.50% <ø> (ø)
unittests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions
Copy link

Metrics Comparison Summary

Total changes across all snapshots: 106

Detailed changes per snapshot

summary_metrics_snapshot_cassandra

📊 Metrics Diff Summary

Total Changes: 53

  • 🆕 Added: 53 metrics
  • ❌ Removed: 0 metrics
  • 🔄 Modified: 0 metrics

🆕 Added Metrics

  • http_server_request_body_size_bytes (18 variants)
View diff sample
+http_server_request_body_size_bytes{http_request_method="GET",http_response_status_code="503",le="+Inf",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
+http_server_request_body_size_bytes{http_request_method="GET",http_response_status_code="503",le="0",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
+http_server_request_body_size_bytes{http_request_method="GET",http_response_status_code="503",le="10",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
+http_server_request_body_size_bytes{http_request_method="GET",http_response_status_code="503",le="100",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
+http_server_request_body_size_bytes{http_request_method="GET",http_response_status_code="503",le="1000",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
+http_server_request_body_size_bytes{http_request_method="GET",http_response_status_code="503",le="10000",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
+http_server_request_body_size_bytes{http_request_method="GET",http_response_status_code="503",le="25",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
...
- `http_server_request_duration_seconds` (17 variants)
View diff sample
+http_server_request_duration_seconds{http_request_method="GET",http_response_status_code="503",le="+Inf",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
+http_server_request_duration_seconds{http_request_method="GET",http_response_status_code="503",le="0.005",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
+http_server_request_duration_seconds{http_request_method="GET",http_response_status_code="503",le="0.01",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
+http_server_request_duration_seconds{http_request_method="GET",http_response_status_code="503",le="0.025",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
+http_server_request_duration_seconds{http_request_method="GET",http_response_status_code="503",le="0.05",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
+http_server_request_duration_seconds{http_request_method="GET",http_response_status_code="503",le="0.075",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
+http_server_request_duration_seconds{http_request_method="GET",http_response_status_code="503",le="0.1",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
...
- `http_server_response_body_size_bytes` (18 variants)
View diff sample
+http_server_response_body_size_bytes{http_request_method="GET",http_response_status_code="503",le="+Inf",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
+http_server_response_body_size_bytes{http_request_method="GET",http_response_status_code="503",le="0",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
+http_server_response_body_size_bytes{http_request_method="GET",http_response_status_code="503",le="10",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
+http_server_response_body_size_bytes{http_request_method="GET",http_response_status_code="503",le="100",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
+http_server_response_body_size_bytes{http_request_method="GET",http_response_status_code="503",le="1000",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
+http_server_response_body_size_bytes{http_request_method="GET",http_response_status_code="503",le="10000",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
+http_server_response_body_size_bytes{http_request_method="GET",http_response_status_code="503",le="25",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
...
### summary_metrics_snapshot_cassandra ## 📊 Metrics Diff Summary

Total Changes: 53

  • 🆕 Added: 0 metrics
  • ❌ Removed: 53 metrics
  • 🔄 Modified: 0 metrics

❌ Removed Metrics

  • http_server_request_body_size_bytes (18 variants)
View diff sample
-http_server_request_body_size_bytes{http_request_method="GET",http_response_status_code="503",le="+Inf",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
-http_server_request_body_size_bytes{http_request_method="GET",http_response_status_code="503",le="0",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
-http_server_request_body_size_bytes{http_request_method="GET",http_response_status_code="503",le="10",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
-http_server_request_body_size_bytes{http_request_method="GET",http_response_status_code="503",le="100",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
-http_server_request_body_size_bytes{http_request_method="GET",http_response_status_code="503",le="1000",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
-http_server_request_body_size_bytes{http_request_method="GET",http_response_status_code="503",le="10000",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
-http_server_request_body_size_bytes{http_request_method="GET",http_response_status_code="503",le="25",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
...
- `http_server_request_duration_seconds` (17 variants)
View diff sample
-http_server_request_duration_seconds{http_request_method="GET",http_response_status_code="503",le="+Inf",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
-http_server_request_duration_seconds{http_request_method="GET",http_response_status_code="503",le="0.005",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
-http_server_request_duration_seconds{http_request_method="GET",http_response_status_code="503",le="0.01",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
-http_server_request_duration_seconds{http_request_method="GET",http_response_status_code="503",le="0.025",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
-http_server_request_duration_seconds{http_request_method="GET",http_response_status_code="503",le="0.05",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
-http_server_request_duration_seconds{http_request_method="GET",http_response_status_code="503",le="0.075",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
-http_server_request_duration_seconds{http_request_method="GET",http_response_status_code="503",le="0.1",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
...
- `http_server_response_body_size_bytes` (18 variants)
View diff sample
-http_server_response_body_size_bytes{http_request_method="GET",http_response_status_code="503",le="+Inf",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
-http_server_response_body_size_bytes{http_request_method="GET",http_response_status_code="503",le="0",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
-http_server_response_body_size_bytes{http_request_method="GET",http_response_status_code="503",le="10",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
-http_server_response_body_size_bytes{http_request_method="GET",http_response_status_code="503",le="100",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
-http_server_response_body_size_bytes{http_request_method="GET",http_response_status_code="503",le="1000",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
-http_server_response_body_size_bytes{http_request_method="GET",http_response_status_code="503",le="10000",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
-http_server_response_body_size_bytes{http_request_method="GET",http_response_status_code="503",le="25",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.63.0",server_address="localhost",server_port="13133",url_scheme="http"}
...

➡️ View full metrics file

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for filtering traces by attributes in the /api/v3/traces HTTP endpoint. Previously, the query.attributes parameter was ignored and always set to an empty map.

  • Implemented parsing of query.attributes JSON parameter in the HTTP handler
  • Added error handling for malformed attributes JSON
  • Included comprehensive unit tests for the new functionality

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
cmd/query/app/apiv3/http_gateway.go Added parsing logic for query.attributes parameter with JSON unmarshaling and error handling
cmd/query/app/apiv3/http_gateway_test.go Added test coverage for attributes parsing and malformed JSON handling

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 226 to 230
}
// Populate the pcommon.Map from the parsed map
for k, v := range attrMap {
attributes.PutStr(k, v)
}
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

The error handling logic is incomplete. If tryParamError writes an error response and returns true, the function continues to execute lines 228-230 which populate the attributes map with data from an uninitialized attrMap. The function should return immediately after the tryParamError block when an error occurs.

Suggested change
}
// Populate the pcommon.Map from the parsed map
for k, v := range attrMap {
attributes.PutStr(k, v)
}
} else {
// Populate the pcommon.Map from the parsed map
for k, v := range attrMap {
attributes.PutStr(k, v)
}
}

Copilot uses AI. Check for mistakes.
ServiceName: q.Get(paramServiceName),
OperationName: q.Get(paramOperationName),
Attributes: pcommon.NewMap(), // most curiously not supported by grpc-gateway
Attributes: attributes,
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

Trailing whitespace detected after the comma. This should be removed to maintain code cleanliness.

Suggested change
Attributes: attributes,
Attributes: attributes,

Copilot uses AI. Check for mistakes.
gw.reader.AssertExpectations(t)
}


Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

Extra blank line should be removed. The file should have only one blank line between function definitions.

Suggested change

Copilot uses AI. Check for mistakes.
Signed-off-by: Divyansh Khatri <[email protected]>
@Divyansh200102 Divyansh200102 changed the title Fix: Support attributes filtering in /api/v3/traces endpoint changelog: Support attributes filtering in /api/v3/traces endpoint Nov 11, 2025
@Divyansh200102
Copy link
Author

Divyansh200102 commented Nov 11, 2025

@yurishkuro I have addressed the feedback .

Comment on lines +218 to +227
var attrMap map[string]string
if err := json.Unmarshal([]byte(attrStr), &attrMap); err != nil {
err := fmt.Errorf("cannot parse attributes JSON: %w", err)
h.tryHandleError(w, err, http.StatusBadRequest)
return attributes, true
}

for k, v := range attrMap {
attributes.PutStr(k, v)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The implementation only supports string-typed attribute values by unmarshaling into map[string]string. This will cause JSON unmarshal errors when users provide valid JSON with numeric or boolean values (e.g., {"http.status_code": 200, "error": true} instead of {"http.status_code": "200", "error": "true"}). Trace attributes commonly include numeric status codes, boolean flags, and other non-string types.

To fix this, unmarshal into map[string]interface{} and use type assertions to handle different value types:

var attrMap map[string]interface{}
if err := json.Unmarshal([]byte(attrStr), &attrMap); err != nil {
    err := fmt.Errorf("cannot parse attributes JSON: %w", err)
    h.tryHandleError(w, err, http.StatusBadRequest)
    return attributes, true
}

for k, v := range attrMap {
    switch val := v.(type) {
    case string:
        attributes.PutStr(k, val)
    case float64:
        attributes.PutDouble(k, val)
    case bool:
        attributes.PutBool(k, val)
    // Add other types as needed
    }
}
Suggested change
var attrMap map[string]string
if err := json.Unmarshal([]byte(attrStr), &attrMap); err != nil {
err := fmt.Errorf("cannot parse attributes JSON: %w", err)
h.tryHandleError(w, err, http.StatusBadRequest)
return attributes, true
}
for k, v := range attrMap {
attributes.PutStr(k, v)
}
var attrMap map[string]interface{}
if err := json.Unmarshal([]byte(attrStr), &attrMap); err != nil {
err := fmt.Errorf("cannot parse attributes JSON: %w", err)
h.tryHandleError(w, err, http.StatusBadRequest)
return attributes, true
}
for k, v := range attrMap {
switch val := v.(type) {
case string:
attributes.PutStr(k, val)
case float64:
attributes.PutDouble(k, val)
case bool:
attributes.PutBool(k, val)
// Add other types as needed
}
}

Spotted by Graphite Agent

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

@Divyansh200102
Copy link
Author

@yurishkuro I have made the changes you asked can you have a look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Support parameters for attributes and limit in api/v3/traces endpoint

2 participants