feat(bigquery): add http tracing telemetry data to Dataset and Model operations#14182
feat(bigquery): add http tracing telemetry data to Dataset and Model operations#14182westarle wants to merge 1 commit intogoogleapis:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces HTTP tracing telemetry for Dataset and Model operations, which is a valuable addition for observability. The implementation is well-structured, centralizing the tracing logic into helper functions. The accompanying tests are thorough, covering various API calls and even retry scenarios, which gives confidence in the correctness of the changes. I have one minor suggestion to improve the robustness of the resource name construction.
bigquery/trace.go
Outdated
| } | ||
|
|
||
| func fullyQualifiedDatasetResourceName(projectID, datasetID string) string { | ||
| if strings.Contains(datasetID, "projects/") { |
There was a problem hiding this comment.
Using strings.Contains to check for a fully qualified name format is a bit fragile. If a dataset ID could validly contain the substring projects/, this would lead to incorrect behavior. While current BigQuery dataset ID constraints make this unlikely, using strings.HasPrefix would be more robust and clearly state the assumption that the fully qualified name segment starts with projects/.
| if strings.Contains(datasetID, "projects/") { | |
| if strings.HasPrefix(datasetID, "projects/") { |
5793d52 to
645badd
Compare
645badd to
3790802
Compare
No description provided.