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

wantLastValueMetric() is not verifying the last value of a metric #623

Open
rhatgadkar-goog opened this issue Sep 18, 2024 · 2 comments · May be fixed by #627
Open

wantLastValueMetric() is not verifying the last value of a metric #623

rhatgadkar-goog opened this issue Sep 18, 2024 · 2 comments · May be fixed by #627
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: cleanup An internal cleanup or hygiene concern.

Comments

@rhatgadkar-goog
Copy link
Contributor

This line passes in TestDialerWithMetrics if the wanted value is set to 1. This is because wantLastValueMetric() is not correctly verifying the last value of a metric. It currently checks whether at some point in time the metric had the wanted value, as seen from these lines.

In TestDialerWithMetrics, the "alloydbconn/open_connections" metric had the following values at different times:

m.name = alloydbconn/open_connections, d.Value = 1
m.name = alloydbconn/open_connections, d.Value = 1
m.name = alloydbconn/open_connections, d.Value = 1
m.name = alloydbconn/open_connections, d.Value = 1
m.name = alloydbconn/open_connections, d.Value = 1
m.name = alloydbconn/open_connections, d.Value = 2

Should we fix wantLastValueMetric() so that it looks at the latest value of the metric?

@enocom
Copy link
Member

enocom commented Sep 19, 2024

Nice find. Yes, let's just fix this.

@enocom enocom assigned rhatgadkar-goog and unassigned enocom Sep 19, 2024
@enocom
Copy link
Member

enocom commented Sep 20, 2024

Once we identify the fix, let's port it over to https://github.com/GoogleCloudPlatform/cloud-sql-go-connector. cc @jackwotherspoon as FYI

@enocom enocom added priority: p2 Moderately-important priority. Fix may not be included in next release. type: cleanup An internal cleanup or hygiene concern. labels Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: cleanup An internal cleanup or hygiene concern.
Projects
None yet
2 participants