feat: endpoint to group spans by trace_id and session_id - #272
Merged
BrianNewsom merged 3 commits intoJun 11, 2026
Merged
Conversation
Signed-off-by: Brian Newsom <brnewsom@nvidia.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds GET /apis/intake/v2/workspaces/{workspace}/spans/groups: required comma-separated by fields, optional pagination/sort/filter; implements domain/schema types, FastAPI handler with validation, service/repository grouping SQL, CLI command, auth mapping, and tests. ChangesSpan Groups Listing
Sequence DiagramsequenceDiagram
participant Client
participant IntakeAPI
participant SpansService
participant SpanRepository
participant ClickHouse
Client->>IntakeAPI: GET /spans/groups?workspace={w}&by=session_id,trace_id&...
IntakeAPI->>SpansService: list_span_groups(filters, group_by, page, page_size, sort)
SpansService->>SpanRepository: list_span_groups(filters, group_by, page, page_size, sort)
SpanRepository->>ClickHouse: execute aggregated GROUP BY SQL (COUNT per group)
ClickHouse-->>SpanRepository: rows (group keys + span_count)
SpanRepository-->>SpansService: PaginatedResult[SpanGroup]
SpansService-->>IntakeAPI: paginated domain groups
IntakeAPI-->>Client: 200 SpanGroupsPage (data, grouped_by, pagination)
Suggested Reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
openapi/openapi.yaml (1)
4105-4112: ⚡ Quick winType
byas CSV array ofSpanGroupByinstead of free-form string.Line 4108 uses
type: string, so OpenAPI can’t enforce allowed values or uniqueness. Model it asarray+explode: false+items: SpanGroupByfor accurate SDK generation and stronger contract validation.Suggested schema improvement
- name: by in: query required: true + explode: false schema: - type: string - description: Comma-separated span fields to group by, e.g. trace_id or session_id,trace_id. + type: array + minItems: 1 + uniqueItems: true + items: + $ref: '`#/components/schemas/SpanGroupBy`' + description: Comma-separated span fields to group by. title: By - description: Comma-separated span fields to group by, e.g. trace_id or session_id,trace_id. + description: Comma-separated span fields to group by.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openapi/openapi.yaml` around lines 4105 - 4112, Change the query parameter "by" from a free-form string to a CSV-style array of the enum type SpanGroupBy so OpenAPI can validate values and uniqueness: replace schema: type: string with schema: type: array and items: { $ref: '`#/components/schemas/SpanGroupBy`' } and add style: form and explode: false on the parameter (keep required: true and descriptions) so SDKs generate a CSV array of SpanGroupBy values instead of an unconstrained string.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openapi/ga/individual/platform.openapi.yaml`:
- Around line 4151-4162: Add a 400 response entry to this operation to cover
invalid/duplicate/empty "by" group-by parameter errors: insert a '400' response
alongside the existing '200' and '422' responses and reference an appropriate
schema (e.g., reuse components.schemas.HTTPValidationError or add a new
components schema like InvalidByParamError) so generated clients and API
contracts reflect the route’s actual behavior; update the operation that
currently references SpanGroupsPage to include this '400' response with
application/json content and the chosen schema.
- Around line 16313-16315: The schema for SpanGroup.group currently forces all
map values to type string ("additionalProperties: type: string"), but group keys
like trace_id can be null; update the additionalProperties schema for
SpanGroup.group to allow nulls (e.g., use type: [ "string", "null" ] or an
equivalent oneOf with string and null) so grouped results with null keys pass
validation; target the SpanGroup.group definition and change its
additionalProperties accordingly.
In `@openapi/ga/openapi.yaml`:
- Around line 4157-4162: The OpenAPI spec currently only documents a '422'
response for validation errors but does not document the '400' bad request
returned for invalid/duplicate/unsupported "by" query values; add a '400'
response object alongside '422' under the same operation and point it to the
same or a suitable schema (e.g., '`#/components/schemas/HTTPValidationError`' or a
new '`#/components/schemas/HTTPBadRequestError`'), include a clear description
like "Bad Request — invalid or unsupported 'by' parameter", and ensure the
operation's responses section lists both '400' and '422' so clients can
distinguish bad requests from validation errors.
- Around line 4105-4112: The query parameter named "by" is declared as a
free-form string but should enforce the allowed values from the SpanGroupBy
enum; update the "by" parameter to be a query array using items: $ref to the
components schema SpanGroupBy (i.e., schema: { type: array, items: { $ref:
'`#/components/schemas/SpanGroupBy`' } }) and set style: form and explode: false
so clients send comma-separated values, and update the description to reflect
it's a comma-separated list of SpanGroupBy enum values.
In `@openapi/openapi.yaml`:
- Around line 4151-4162: Add a 400 response to the OpenAPI responses for the
operation that returns SpanGroupsPage to represent invalid/duplicate "by"
parameter values: update the responses block alongside the existing '200' and
'422' entries to include a '400' entry with a description like "Invalid 'by'
parameter" and point its content schema to the existing HTTPValidationError (or
a new HTTPError schema if preferred) so clients can reliably handle semantic
validation errors for the "by" query parameter.
---
Nitpick comments:
In `@openapi/openapi.yaml`:
- Around line 4105-4112: Change the query parameter "by" from a free-form string
to a CSV-style array of the enum type SpanGroupBy so OpenAPI can validate values
and uniqueness: replace schema: type: string with schema: type: array and items:
{ $ref: '`#/components/schemas/SpanGroupBy`' } and add style: form and explode:
false on the parameter (keep required: true and descriptions) so SDKs generate a
CSV array of SpanGroupBy values instead of an unconstrained string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 2bab772e-f3b9-4705-b8d4-c26db13dec78
⛔ Files ignored due to path filters (16)
sdk/python/nemo-platform/.nmpcontext/openapi.yamlis excluded by!sdk/**sdk/python/nemo-platform/.nmpcontext/stainless.yamlis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/api/intake/spans/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/api/intake/spans/groups.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/intake/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/intake/spans/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/intake/spans/groups.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/intake/spans/spans.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/intake/spans/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/intake/spans/group_list_params.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/intake/spans/span_group.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/intake/spans/span_group_by.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/intake/spans/span_group_sort_field.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/intake/spans/span_groups_page.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/intake/spans/test_groups.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (15)
openapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/api/intake/spans/__init__.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/api/intake/spans/groups.pyservices/core/auth/src/nmp/core/auth/assets/static-authz.yamlservices/intake/README.mdservices/intake/src/nmp/intake/spans/api/spans.pyservices/intake/src/nmp/intake/spans/api/spans_schemas.pyservices/intake/src/nmp/intake/spans/domain.pyservices/intake/src/nmp/intake/spans/service.pyservices/intake/src/nmp/intake/spans/span_repository.pyservices/intake/tests/integration/spans/test_spans_read_filters.pyservices/intake/tests/test_spans_clickhouse_repository.pyservices/intake/tests/test_spans_schemas.py
Contributor
|
Signed-off-by: Brian Newsom <brnewsom@nvidia.com>
…n-queries-to-sessions
shanaiabuggy
approved these changes
Jun 11, 2026
This was referenced Jul 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit