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

Conform sqlalchemy to prom spec #804

Merged
merged 2 commits into from
Mar 14, 2024
Merged

Conform sqlalchemy to prom spec #804

merged 2 commits into from
Mar 14, 2024

Conversation

nkall
Copy link
Contributor

@nkall nkall commented Sep 5, 2023

💸 TL;DR

The SqlAlchemy library is old and emits events not compliant with the prometheus metrics spec. Specifically, sql_client_name is missing and instead this library provides sql_pool which appeared in an older version of the spec. This updates the metrics to be compliant with spec and usable by the baseplate service health dashboard.

📜 Details

Example SQL metrics emitted by a client of this library:

$ curl -s localhost:6060/metrics | grep sql
# HELP sql_client_latency_seconds Latency histogram of calls to database
# TYPE sql_client_latency_seconds histogram
sql_client_latency_seconds_sum{sql_address="postgresql",sql_database="reputation",sql_pool="sqlalchemy",sql_success="true"} 0.004516069000601419
sql_client_latency_seconds_bucket{le="0.0001",sql_address="postgresql",sql_database="reputation",sql_pool="sqlalchemy",sql_success="true"} 0.0
sql_client_latency_seconds_bucket{le="0.0005",sql_address="postgresql",sql_database="reputation",sql_pool="sqlalchemy",sql_success="true"} 0.0
sql_client_latency_seconds_bucket{le="0.001",sql_address="postgresql",sql_database="reputation",sql_pool="sqlalchemy",sql_success="true"} 2.0
sql_client_latency_seconds_bucket{le="0.0025",sql_address="postgresql",sql_database="reputation",sql_pool="sqlalchemy",sql_success="true"} 4.0
sql_client_latency_seconds_bucket{le="0.005",sql_address="postgresql",sql_database="reputation",sql_pool="sqlalchemy",sql_success="true"} 4.0
sql_client_latency_seconds_bucket{le="0.01",sql_address="postgresql",sql_database="reputation",sql_pool="sqlalchemy",sql_success="true"} 4.0
sql_client_latency_seconds_bucket{le="0.025",sql_address="postgresql",sql_database="reputation",sql_pool="sqlalchemy",sql_success="true"} 4.0
sql_client_latency_seconds_bucket{le="0.05",sql_address="postgresql",sql_database="reputation",sql_pool="sqlalchemy",sql_success="true"} 4.0
sql_client_latency_seconds_bucket{le="0.1",sql_address="postgresql",sql_database="reputation",sql_pool="sqlalchemy",sql_success="true"} 4.0
sql_client_latency_seconds_bucket{le="0.25",sql_address="postgresql",sql_database="reputation",sql_pool="sqlalchemy",sql_success="true"} 4.0
sql_client_latency_seconds_bucket{le="0.5",sql_address="postgresql",sql_database="reputation",sql_pool="sqlalchemy",sql_success="true"} 4.0
sql_client_latency_seconds_bucket{le="1.0",sql_address="postgresql",sql_database="reputation",sql_pool="sqlalchemy",sql_success="true"} 4.0
sql_client_latency_seconds_bucket{le="5.0",sql_address="postgresql",sql_database="reputation",sql_pool="sqlalchemy",sql_success="true"} 4.0
sql_client_latency_seconds_bucket{le="15.0",sql_address="postgresql",sql_database="reputation",sql_pool="sqlalchemy",sql_success="true"} 4.0
sql_client_latency_seconds_bucket{le="30.0",sql_address="postgresql",sql_database="reputation",sql_pool="sqlalchemy",sql_success="true"} 4.0
sql_client_latency_seconds_bucket{le="+Inf",sql_address="postgresql",sql_database="reputation",sql_pool="sqlalchemy",sql_success="true"} 4.0
sql_client_latency_seconds_count{sql_address="postgresql",sql_database="reputation",sql_pool="sqlalchemy",sql_success="true"} 4.0
# HELP sql_client_requests_total Total number of sql requests
# TYPE sql_client_requests_total counter
sql_client_requests_total{sql_address="postgresql",sql_database="reputation",sql_pool="sqlalchemy",sql_success="true"} 4.0
# HELP sql_client_active_requests total requests that are in-flight
# TYPE sql_client_active_requests gauge
sql_client_active_requests{sql_address="postgresql",sql_database="reputation",sql_pool="sqlalchemy"} 0.0

🧪 Testing Steps / Validation

I will require this branch in karma service and verify in my local snoodev.

✅ Checks

  • CI tests (if present) are passing
  • Adheres to code style for repo
  • Contributor License Agreement (CLA) completed if not a Reddit employee

@nkall nkall requested a review from a team as a code owner September 5, 2023 19:22
@nkall nkall requested a review from KTAtkinson September 5, 2023 19:22
Copy link
Contributor

@KTAtkinson KTAtkinson left a comment

Choose a reason for hiding this comment

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

It looks like there are a few more tests that need to be updated.

@nkall nkall requested a review from KTAtkinson September 12, 2023 18:23
Copy link
Contributor

@KTAtkinson KTAtkinson left a comment

Choose a reason for hiding this comment

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

This is a breaking change!

@KTAtkinson KTAtkinson added the breaking change This change contains a breaking change label Sep 15, 2023
@nkall
Copy link
Contributor Author

nkall commented Sep 15, 2023

Ty so much!

@chriskuehl chriskuehl merged commit 0c6d086 into reddit:develop Mar 14, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change This change contains a breaking change
Development

Successfully merging this pull request may close these issues.

3 participants