Implement a metrics Prometheus endpoint - #1242
Conversation
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
There was a problem hiding this comment.
Benchmark Index (community)
Details
| Benchmark suite | Current: 7e1de02 | Previous: 4b1d28e | Ratio |
|---|---|---|---|
Add one schema (0 existing) |
309 ms |
289 ms |
1.07 |
Add one schema (100 existing) |
37 ms |
45 ms |
0.82 |
Add one schema (1000 existing) |
95 ms |
103 ms |
0.92 |
Add one schema (10000 existing) |
1147 ms |
857 ms |
1.34 |
Update one schema (1 existing) |
27 ms |
29 ms |
0.93 |
Update one schema (101 existing) |
38 ms |
38 ms |
1 |
Update one schema (1001 existing) |
105 ms |
106 ms |
0.99 |
Update one schema (10001 existing) |
1015 ms |
867 ms |
1.17 |
Cached rebuild (1 existing) |
10 ms |
8 ms |
1.25 |
Cached rebuild (101 existing) |
12 ms |
10 ms |
1.20 |
Cached rebuild (1001 existing) |
46 ms |
28 ms |
1.64 |
Cached rebuild (10001 existing) |
352 ms |
206 ms |
1.71 |
Index 100 schemas |
642 ms |
605 ms |
1.06 |
Index 1000 schemas |
1447 ms |
1236 ms |
1.17 |
Index 10000 schemas |
12878 ms |
13239 ms |
0.97 |
Index 10000 schemas (custom meta-schema) |
15181 ms |
16038 ms |
0.95 |
Index 10000 schemas ($ref fan-out) |
15066 ms |
15520 ms |
0.97 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Benchmark Index (enterprise)
Details
| Benchmark suite | Current: 7e1de02 | Previous: 4b1d28e | Ratio |
|---|---|---|---|
Add one schema (0 existing) |
378 ms |
376 ms |
1.01 |
Add one schema (100 existing) |
127 ms |
126 ms |
1.01 |
Add one schema (1000 existing) |
180 ms |
187 ms |
0.96 |
Add one schema (10000 existing) |
822 ms |
823 ms |
1.00 |
Update one schema (1 existing) |
113 ms |
117 ms |
0.97 |
Update one schema (101 existing) |
127 ms |
122 ms |
1.04 |
Update one schema (1001 existing) |
180 ms |
187 ms |
0.96 |
Update one schema (10001 existing) |
837 ms |
839 ms |
1.00 |
Cached rebuild (1 existing) |
12 ms |
12 ms |
1 |
Cached rebuild (101 existing) |
18 ms |
15 ms |
1.20 |
Cached rebuild (1001 existing) |
45 ms |
45 ms |
1 |
Cached rebuild (10001 existing) |
358 ms |
363 ms |
0.99 |
Index 100 schemas |
553 ms |
481 ms |
1.15 |
Index 1000 schemas |
1537 ms |
1526 ms |
1.01 |
Index 10000 schemas |
13042 ms |
12951 ms |
1.01 |
Index 10000 schemas (custom meta-schema) |
15204 ms |
15426 ms |
0.99 |
Index 10000 schemas ($ref fan-out) |
15642 ms |
15728 ms |
0.99 |
This comment was automatically generated by workflow using github-action-benchmark.
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
🤖 Augment PR SummarySummary: This PR introduces a Prometheus-compatible metrics endpoint for Sourcemeta One.
🤖 Was this summary useful? React with 👍 or 👎 |
| auto snapshot = std::make_shared<HTTPRequest>( | ||
| std::string{this->method()}, std::string{this->path()}, | ||
| this->response_encoding_, raw_response); | ||
| snapshot->observation_ = this->observation_; |
There was a problem hiding this comment.
src/http/include/sourcemeta/one/http_request.h:182: a request aborted while body() is waiting never calls Observation::record()—the onAborted handler only flips completed. Since construction has already called enter(), each such disconnect permanently raises sourcemeta_one_http_requests_in_flight, so its value becomes unreliable and can grow without bound under aborted uploads.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
3 issues found across 22 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/http/include/sourcemeta/one/http_request.h">
<violation number="1" location="src/http/include/sourcemeta/one/http_request.h:53">
P2: When a handler throws or an asynchronous body request is aborted, this increment is never paired with `Observation::record()`. The Prometheus `in_flight` value then remains permanently elevated and the request is missing from answered totals; centralize completion with an idempotent guard that records error and abort paths too.</violation>
</file>
<file name="src/http/include/sourcemeta/one/http_metrics.h">
<violation number="1" location="src/http/include/sourcemeta/one/http_metrics.h:64">
P3: The exported `process_start_time_seconds` is recorded when `Router` calls `start()`, after process initialization and route loading. Capture the timestamp at process/global initialization, or rename the metric to represent server start.</violation>
</file>
<file name="enterprise/e2e/auth/prometheus.yml">
<violation number="1" location="enterprise/e2e/auth/prometheus.yml:15">
P3: The metrics bearer credential is duplicated as a literal in three files (`environment`, `prometheus.yml`, and every `Authorization` header in `metrics.all.hurl`) instead of being sourced from the single `ONE_E2E_KEY_METRICS` definition. Rotating the secret requires editing every site, and a mismatch breaks the scraper or the tests without an obvious cause. Inject the value into the prometheus service (e.g. `env_file: environment` plus `credentials: ${ONE_E2E_KEY_METRICS}`) so it follows the env-var convention already used by the other keys.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| : request_{request}, response_{response} {} | ||
| : request_{request}, response_{response} { | ||
| this->observation_.started = std::chrono::steady_clock::now(); | ||
| http_metrics().enter(); |
There was a problem hiding this comment.
P2: When a handler throws or an asynchronous body request is aborted, this increment is never paired with Observation::record(). The Prometheus in_flight value then remains permanently elevated and the request is missing from answered totals; centralize completion with an idempotent guard that records error and abort paths too.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/http/include/sourcemeta/one/http_request.h, line 53:
<comment>When a handler throws or an asynchronous body request is aborted, this increment is never paired with `Observation::record()`. The Prometheus `in_flight` value then remains permanently elevated and the request is missing from answered totals; centralize completion with an idempotent guard that records error and abort paths too.</comment>
<file context>
@@ -25,12 +28,30 @@ namespace sourcemeta::one {
- : request_{request}, response_{response} {}
+ : request_{request}, response_{response} {
+ this->observation_.started = std::chrono::steady_clock::now();
+ http_metrics().enter();
+ }
</file context>
| // counted before this is said | ||
| auto start(const std::size_t handlers) -> void { | ||
| this->handlers_ = handlers; | ||
| this->started_ = |
There was a problem hiding this comment.
P3: The exported process_start_time_seconds is recorded when Router calls start(), after process initialization and route loading. Capture the timestamp at process/global initialization, or rename the metric to represent server start.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/http/include/sourcemeta/one/http_metrics.h, line 64:
<comment>The exported `process_start_time_seconds` is recorded when `Router` calls `start()`, after process initialization and route loading. Capture the timestamp at process/global initialization, or rename the metric to represent server start.</comment>
<file context>
@@ -0,0 +1,206 @@
+ // counted before this is said
+ auto start(const std::size_t handlers) -> void {
+ this->handlers_ = handlers;
+ this->started_ =
+ std::chrono::duration<double>{
+ std::chrono::system_clock::now().time_since_epoch()}
</file context>
| # like any other caller | ||
| authorization: | ||
| type: Bearer | ||
| credentials: metrics-secret-key |
There was a problem hiding this comment.
P3: The metrics bearer credential is duplicated as a literal in three files (environment, prometheus.yml, and every Authorization header in metrics.all.hurl) instead of being sourced from the single ONE_E2E_KEY_METRICS definition. Rotating the secret requires editing every site, and a mismatch breaks the scraper or the tests without an obvious cause. Inject the value into the prometheus service (e.g. env_file: environment plus credentials: ${ONE_E2E_KEY_METRICS}) so it follows the env-var convention already used by the other keys.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At enterprise/e2e/auth/prometheus.yml, line 15:
<comment>The metrics bearer credential is duplicated as a literal in three files (`environment`, `prometheus.yml`, and every `Authorization` header in `metrics.all.hurl`) instead of being sourced from the single `ONE_E2E_KEY_METRICS` definition. Rotating the secret requires editing every site, and a mismatch breaks the scraper or the tests without an obvious cause. Inject the value into the prometheus service (e.g. `env_file: environment` plus `credentials: ${ONE_E2E_KEY_METRICS}`) so it follows the env-var convention already used by the other keys.</comment>
<file context>
@@ -0,0 +1,18 @@
+ # like any other caller
+ authorization:
+ type: Bearer
+ credentials: metrics-secret-key
+ static_configs:
+ - targets:
</file context>
Signed-off-by: Juan Cruz Viotti jv@jviotti.com