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

Capture and update search metrics for TraceQL #2087

Merged
merged 12 commits into from
Apr 11, 2023

Conversation

electron0zero
Copy link
Member

@electron0zero electron0zero commented Feb 8, 2023

What this PR does:

This PR makes following changes:

why?
InspectedBytes metric from in SearchMetrics is used to compute throughput and SLO metrics (added in #2008 ), To accurately compute throughput, we need accurate size of data read.

Which issue(s) this PR fixes:
Fixes #2086

Checklist

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

}

// combine iters?
return traceql.FetchSpansResponse{
Results: &mergeSpansetIterator{
iters: iters,
},
Bytes: func() uint64 {
Copy link
Member Author

Choose a reason for hiding this comment

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

cc @stoewer because I am making same changes in vparquet2 format as well.

Copy link
Contributor

@mdisibio mdisibio left a comment

Choose a reason for hiding this comment

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

The wal metrics lgtm, but I would like @stoewer to review the vparquet2 DurationNanos change.

tempodb/encoding/vparquet2/block_search.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@stoewer stoewer left a comment

Choose a reason for hiding this comment

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

The vParquet2 changes look good to me

I still added a couple of minor suggestions

tempodb/encoding/vparquet/readers.go Outdated Show resolved Hide resolved
tempodb/encoding/vparquet/readers.go Outdated Show resolved Hide resolved
tempodb/encoding/vparquet/wal_block.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@stoewer stoewer left a comment

Choose a reason for hiding this comment

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

LGTM

@joe-elliott joe-elliott merged commit c0a0d4b into grafana:main Apr 11, 2023
@electron0zero electron0zero deleted the traceql_metrics branch April 11, 2023 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

SearchMetrics is always empty for TraceQL queries
4 participants