metrics

[brandtnewton]

No description provided.

[gemini-code-assist]

Code Review

This pull request introduces detailed OpenTelemetry latency metrics (parsing, bigtable, and processing latencies) across various database operations in the Cassandra-Bigtable proxy. The feedback suggests simplifying the metric recording functions (RecordParsingLatencyMetric, RecordBigtableLatencyMetric, and RecordProcessingLatencyMetric) to accept a time.Duration directly instead of a time.Time start time. This change would eliminate convoluted calculations like time.Now().Add(...) at the call sites, significantly improving code readability and maintainability.

[gemini-code-assist]

The metric recording functions RecordParsingLatencyMetric, RecordBigtableLatencyMetric, and RecordProcessingLatencyMetric currently accept a startTime time.Time and calculate the duration internally using time.Since(startTime).

However, in several places (such as bigtable.go and bigtable_select.go), the processing/Bigtable durations are calculated as split durations or differences, requiring convoluted workarounds like time.Now().Add(-(processingDuration + time.Since(procStart))) to pass a synthetic start time.

Changing these functions to accept a time.Duration directly simplifies the call sites significantly, improves readability, and avoids unnecessary clock calculations.

// RecordParsingLatencyMetric() records the latency of the query parsing phase.
func (o *OpenTelemetry) RecordParsingLatencyMetric(ctx context.Context, duration time.Duration, attrs Attributes) {
	if !o.Config.OTELEnabled {
		return
	}

	attr := []attribute.KeyValue{
		attributeKeyInstance.String(attrs.Keyspace),
		attributeKeyDatabase.String(o.Config.Database),
		attributeKeyMethod.String(attrs.Method),
		attributeKeyQueryType.String(attrs.QueryType),
	}
	o.parsingLatency.Record(ctx, int64(duration.Milliseconds()), metric.WithAttributes(attr...))
}

// RecordBigtableLatencyMetric() records the latency of the Bigtable call phase.
func (o *OpenTelemetry) RecordBigtableLatencyMetric(ctx context.Context, duration time.Duration, attrs Attributes) {
	if !o.Config.OTELEnabled {
		return
	}

	attr := []attribute.KeyValue{
		attributeKeyInstance.String(attrs.Keyspace),
		attributeKeyDatabase.String(o.Config.Database),
		attributeKeyMethod.String(attrs.Method),
		attributeKeyQueryType.String(attrs.QueryType),
	}
	o.bigtableLatency.Record(ctx, int64(duration.Milliseconds()), metric.WithAttributes(attr...))
}

// RecordProcessingLatencyMetric() records the latency of the result processing phase.
func (o *OpenTelemetry) RecordProcessingLatencyMetric(ctx context.Context, duration time.Duration, attrs Attributes) {
	if !o.Config.OTELEnabled {
		return
	}

	attr := []attribute.KeyValue{
		attributeKeyInstance.String(attrs.Keyspace),
		attributeKeyDatabase.String(o.Config.Database),
		attributeKeyMethod.String(attrs.Method),
		attributeKeyQueryType.String(attrs.QueryType),
	}
	o.processingLatency.Record(ctx, int64(duration.Milliseconds()), metric.WithAttributes(attr...))
}

References

1. Prioritize code readability over minor performance optimizations. A less efficient but more readable implementation may be preferred over a more efficient but less clear one.

[gemini-code-assist]

By changing RecordProcessingLatencyMetric to accept a time.Duration directly, we can avoid the convoluted time.Now().Add(...) calculation and pass the accumulated duration directly.

Suggested change

	procStart := time.Now()
	res := GenerateAppliedRowsResult(input.Keyspace(), input.Table(), input.IfSpec.IfExists == matched)
	btc.otelInst.RecordProcessingLatencyMetric(ctx, time.Now().Add(-(processingDuration + time.Since(procStart))), otelgo.Attributes{
	Method: "mutateRow",
	QueryType: input.QueryType().String(),
	Keyspace: string(input.Keyspace()),
	})
	return res, nil
	procStart := time.Now()
	res := GenerateAppliedRowsResult(input.Keyspace(), input.Table(), input.IfSpec.IfExists == matched)
	btc.otelInst.RecordProcessingLatencyMetric(ctx, processingDuration+time.Since(procStart), otelgo.Attributes{
	Method: "mutateRow",
	QueryType: input.QueryType().String(),
	Keyspace: string(input.Keyspace()),
	})
	return res, nil

References

1. Prioritize code readability over minor performance optimizations. A less efficient but more readable implementation may be preferred over a more efficient but less clear one.

[gemini-code-assist]

By changing the OTel latency recording functions to accept time.Duration directly, we can avoid using time.Now().Add(-duration) to pass synthetic start times, making this block much cleaner and easier to read.

	buildResponseStart := time.Now()
	rowsResult, buildErr := responsehandler.BuildRowsResultResponse(query, rows, query.ProtocolVersion)
	processingDuration += time.Since(buildResponseStart)

	bigtableLatency := totalExecuteDuration - processingDuration
	if bigtableLatency < 0 {
		bigtableLatency = 0
	}

	btc.otelInst.RecordBigtableLatencyMetric(ctx, bigtableLatency, otelgo.Attributes{
		Method:    "ExecutePreparedStatement",
		QueryType: query.QueryType().String(),
		Keyspace:  string(query.Keyspace()),
	})
	btc.otelInst.RecordProcessingLatencyMetric(ctx, processingDuration, otelgo.Attributes{
		Method:    "ExecutePreparedStatement",
		QueryType: query.QueryType().String(),
		Keyspace:  string(query.Keyspace()),
	})

	return rowsResult, buildErr

References

1. Prioritize code readability over minor performance optimizations. A less efficient but more readable implementation may be preferred over a more efficient but less clear one.