Skip to content

perf(bigtable): Refactor metric attributes for performance #12445

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.

Sign up for GitHub

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

Jump to bottom
Merged
bhshkh merged 2 commits into from
Jun 19, 2025
Merged

perf(bigtable): Refactor metric attributes for performance #12445

bhshkh merged 2 commits into from
Jun 19, 2025

Conversation

@bhshkh
Copy link
Contributor

@bhshkh bhshkh commented Jun 13, 2025 •
edited
Loading

Documentation of WithAttributes is :

It converts attributes into an attribute Set and sets the Set to be associated with a measurement. This is shorthand for:

cp := make([]attribute.KeyValue, len(attributes)) 
copy(cp, attributes) 
WithAttributeSet(attribute.NewSet(cp...))

attribute.NewSet may modify the passed attributes so this will make a copy of attributes before creating a set in order to ensure this function is concurrent safe. This makes this option function less optimized in comparison to WithAttributeSet. Therefore, WithAttributeSet should be preferred for performance sensitive code.

In this PR, the toOtelMetricAttrs function in bigtable/metrics.go has been modified to return an attribute.Set instead of a []attribute.KeyValue slice. This allows for more efficient attribute handling.
So, earlier

toOtelMetricAttrs creates []attribute.KeyValue
WithAttributes makes a copy of the above attributes
WithAttributes creates attribute set using the above copied attributes

Now,

toOtelMetricAttrs creates []attribute.KeyValue
toOtelMetricAttrs creates attribute set from []attribute.KeyValue
WithAttributeSet uses the attribute set

This should lead to performance gain since the copy step is eliminated

Below are the pprof flame graphs of 10 Go routines reading a million rows. Top is without the changes in this PR and bottom is with changes in this PR.
Screenshot 2025-06-15 at 2 01 05 AM
Screenshot 2025-06-15 at 2 01 09 AM
In this run, recordOperationCompletion took 16.36% lesser time.

Repeated the same again
Screenshot 2025-06-15 at 2 01 18 AM
Screenshot 2025-06-15 at 2 01 21 AM
In this run, recordOperationCompletion took 17.97% lesser time.

@product-auto-label product-auto-label bot added the api: bigtable Issues related to the Bigtable API. label Jun 13, 2025
@bhshkh bhshkh marked this pull request as ready for review June 15, 2025 09:06
@bhshkh bhshkh requested review from a team as code owners June 15, 2025 09:06
@bhshkh bhshkh force-pushed the perf-metric-attrs branch from a64df39 to c7c7c0c Compare June 15, 2025 09:13
@bhshkh bhshkh enabled auto-merge (squash) June 15, 2025 09:14
gkevinzheng
gkevinzheng approved these changes Jun 17, 2025
View reviewed changes
Copy link
Contributor

@gkevinzheng gkevinzheng left a comment

Choose a reason for hiding this comment

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

LGTM

@bhshkh bhshkh merged commit f734ec6 into googleapis:main Jun 19, 2025
11 checks passed
@bhshkh bhshkh deleted the perf-metric-attrs branch June 19, 2025 13:35
@release-please release-please bot mentioned this pull request Jun 18, 2025
Merged
qiaodev pushed a commit to qiaodev/google-cloud-go that referenced this pull request Jun 23, 2025
@bhshkh @qiaodev
perf(bigtable): Refactor metric attributes for performance (googleapi…
17aaa92 
…s#12445)
This was linked to issues Sep 30, 2025
bigtable: Support client side metrics #12997
Closed
bigtable: Client side metrics performance improvements #12998
Closed
This was unlinked from issues Sep 30, 2025
bigtable: Support client side metrics #12997
Closed
bigtable: Client side metrics performance improvements #12998
Closed
@bhshkh bhshkh linked an issue Sep 30, 2025 that may be closed by this pull request
bigtable: Client side metrics performance improvements #12998
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gkevinzheng gkevinzheng gkevinzheng approved these changes

Assignees

No one assigned

Labels

api: bigtable Issues related to the Bigtable API.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

bigtable: Client side metrics performance improvements

2 participants

@bhshkh @gkevinzheng