Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[query] Add series metadata label limit header for trimming response #3348

Merged

Conversation

robskillington
Copy link
Collaborator

What this PR does / why we need it:

Adds ability to trim series metadata / label metadata via header.

Special notes for your reviewer:

Does this PR introduce a user-facing and/or backwards incompatible change?:

NONE

Does this PR require updating code package or user-facing documentation?:

NONE

TotalSeries: returnedDataLimited.TotalSeries,
Datapoints: returnedDataLimited.Datapoints,
}
err = handleroptions.AddResponseHeaders(w, resultMetadata, fetchOptions, limited, nil)
Copy link
Collaborator

Choose a reason for hiding this comment

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

so we're always adding the response header now, 👍

cc @rallen090

ReturnedSeriesMetadataLimit: opts.ReturnedSeriesMetadataLimit,
}
)
renderResult, err := prometheus.RenderSeriesMatchResultsJSON(noopWriter, results, renderOpts)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we leave comments on these noop render calls to explain the purpose is to apply the limiting first so we can write the header before the actual body writing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah sg.

total += len(tag.Values)
if opts.ReturnedSeriesMetadataLimit > 0 && rendered >= opts.ReturnedSeriesMetadataLimit {
limited = true
continue
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok I see - so we continue here so we can still count total

Copy link
Collaborator

@rallen090 rallen090 left a comment

Choose a reason for hiding this comment

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

LGTM

@robskillington robskillington merged commit 495fbed into master Mar 11, 2021
@robskillington robskillington deleted the r/add-series-metadata-label-limit-header-response branch March 11, 2021 19:57
soundvibe added a commit that referenced this pull request Mar 15, 2021
* master:
  [dbnode] Remove unused getMockReader (#3351)
  [dbnode] Add back teardown of containers at end of Prometheus integration test (#3349)
  [query] Add series metadata label limit header for trimming response (#3348)

# Conflicts:
#	src/dbnode/storage/shard_test.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants