Skip to content

fix: skip exemplars for instant queries#4204

Merged
javiermolinar merged 9 commits intografana:mainfrom
javiermolinar:skip-exemplars-for-instant-queries
Oct 25, 2024
Merged

fix: skip exemplars for instant queries#4204
javiermolinar merged 9 commits intografana:mainfrom
javiermolinar:skip-exemplars-for-instant-queries

Conversation

@javiermolinar
Copy link
Copy Markdown
Contributor

@javiermolinar javiermolinar commented Oct 18, 2024

What this PR does:
Currently, we are computing the exemplars for query_range and instant queries. Since exemplars are not supported for instant queries (same as Prometheus) we can save that computing time.

It also fix a bug where the exemplars query param was not being honored

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@javiermolinar
Copy link
Copy Markdown
Contributor Author

Can we live without this config?

Exemplars: false, // TODO: Remove?

If max_exemplars is set to 0 then it means that the Exemplars are disabled

@javiermolinar javiermolinar added the type/docs Improvements or additions to documentation label Oct 18, 2024
@mdisibio
Copy link
Copy Markdown
Contributor

Can we live without this config?

Exemplars: false, // TODO: Remove?

If max_exemplars is set to 0 then it means that the Exemplars are disabled

This sounds good to me. @mapno You originally implemented this, can you think of a scenario where max_exemplars=0 and exemplars=true is meaningful?

Comment thread docs/sources/tempo/traceql/metrics-queries.md Outdated
Comment thread modules/frontend/metrics_query_range_sharder.go
Co-authored-by: Martin Disibio <mdisibio@gmail.com>
@mapno
Copy link
Copy Markdown
Contributor

mapno commented Oct 22, 2024

Can we live without this config?

Exemplars: false, // TODO: Remove?

If max_exemplars is set to 0 then it means that the Exemplars are disabled

This sounds good to me. @mapno You originally implemented this, can you think of a scenario where max_exemplars=0 and exemplars=true is meaningful?

Can't think of one. Possible was left from a previous implementation. I'm good to remove it.

Copy link
Copy Markdown
Contributor

@knylander-grafana knylander-grafana left a comment

Choose a reason for hiding this comment

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

Thank you for updating the docs. The updates look good.

@javiermolinar javiermolinar merged commit de5cb00 into grafana:main Oct 25, 2024
@javiermolinar javiermolinar deleted the skip-exemplars-for-instant-queries branch October 25, 2024 19:48
knylander-grafana pushed a commit that referenced this pull request Oct 29, 2024
fix: skip exemplars for instant queries (#4204)

Currently, we are computing the exemplars for query_range and  instant queries. Since exemplars are not supported for instant queries (same as Prometheus) we can save that computing time.

It also fixes a bug where the exemplars query param was not being honored

Co-authored-by: Martin Disibio <mdisibio@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/docs Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants