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

feat(telemetry): adds reads and writes #25409

Merged
merged 1 commit into from
Oct 1, 2024

Conversation

praveen-influx
Copy link
Contributor

  • instrumented code to get read and write measurement
  • introduced EventsBucket for collection of reads/writes
  • sampler now samples every minute for all metrics (including reads/writes)
  • other tidy ups

closes: #25372

@praveen-influx praveen-influx force-pushed the praveen/telem-writes-report branch 11 times, most recently from 818eed9 to ad415c4 Compare October 1, 2024 15:27
@praveen-influx praveen-influx marked this pull request as ready for review October 1, 2024 15:29
Copy link
Contributor

@hiltontj hiltontj left a comment

Choose a reason for hiding this comment

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

Just one small thing for deps cleanup.

Comment on lines 20 to 21
tonic.workspace = true
tower.workspace = true
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these used since moving away from the middleware/interceptor approach?

let num_lines = result.line_count;
let payload_size = body.len();
let telem_store = Arc::clone(&self.common_state.telemetry_store);
tokio::spawn(async move {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we want to spawn on every write call. We should be able to just call add_write_metrics on the store without this.

@@ -146,6 +151,13 @@ impl QueryExecutor for QueryExecutorImpl {
let token = token.permit();

debug!("execute stream of query results");

let telem_clone = Arc::clone(&self.telemetry_store);
tokio::spawn(async move {
Copy link
Member

Choose a reason for hiding this comment

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

Same as before, we don't want to spawn on every query.

- instrumented code to get read and write measurement
- introduced EventsBucket for collection of reads/writes
- sampler now samples every minute for all metrics (including
  reads/writes)
- other tidy ups

closes: #25372
@praveen-influx praveen-influx merged commit 72dcd18 into main Oct 1, 2024
12 checks passed
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.

Send telemetry report for reads and writes
3 participants