chore: Add MetricsRecorder interface to Datastore#12028
Conversation
|
|
||
| this.transactionLatency = | ||
| meter | ||
| .histogramBuilder(TelemetryConstants.SERVICE_NAME + "/transaction_latency") |
There was a problem hiding this comment.
Do we need /internal/client prefixes?
There was a problem hiding this comment.
I'm not 100% sure about this long-term. I think I would need to run this by the platform team. For now, since it's not being sent, I think this prefix should be fine. Since we're not promoting this yet, I think we should be able to change this in the future.
| public static final String ATTRIBUTES_KEY_MORE_RESULTS = "more_results"; | ||
|
|
||
| /* TODO(lawrenceqiu): For now, these are a duplicate of method names in TraceUtil. Those will use these eventually */ | ||
| public static final String METHOD_ALLOCATE_IDS = "AllocateIds"; |
There was a problem hiding this comment.
I noticed that these are PascalCase, while others are lowercase (e.g. "add", "put"). Are there plans to standardize them?
There was a problem hiding this comment.
Yeah, this isn't consistent and I think ideally it should be in pascalcase. These constants we pulled from the existing TraceUtil file. I don't want to change this since there may be customers who are currently using those traces.
2 participants
Adds the Otel Metrics Recording Implementation as well as the No-Op Metrics Recording implementation for Datastore.
The implementation will be chosen based on the opt-in or opt-out choice for Metrics recording.