Skip to content

Implements Python startup and internal error metrics for HMD.#6603

Merged
dom96 merged 1 commit intomainfrom
dominik/python-hmd-metrics
Apr 22, 2026
Merged

Implements Python startup and internal error metrics for HMD.#6603
dom96 merged 1 commit intomainfrom
dominik/python-hmd-metrics

Conversation

@dom96
Copy link
Copy Markdown
Contributor

@dom96 dom96 commented Apr 17, 2026

Just some scaffolding to help count Python internal error metrics. Tested upstream.

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented Apr 17, 2026

I've now reviewed all the changed files thoroughly. Let me analyze the PR:

Changes summary:

  1. observer.h: Adds setPythonWorkersInternalError() virtual method to RequestObserver
  2. pyodide.h / pyodide.c++: Adds reportPythonWorkersInternalError JSG method to WorkerFatalReporter that calls the observer
  3. util.ts: PythonWorkersInternalError constructor now calls FatalReporter.reportPythonWorkersInternalError() (with try/catch)
  4. python-entrypoint-helper.ts: Adds TEST_ONLY_THROW_PYTHON_WORKERS_INTERNAL_ERROR helper gated behind experimental flag
  5. Type declarations updated for the new method and experimental compat flag

Let me verify that this follows existing patterns correctly and check for any issues:

  • The C++ pattern follows the exact same structure as reportFatal/setWorkerFatal — consistent.
  • The try/catch in the constructor is appropriate since reporting metrics should not break error construction.
  • The observer method has a no-op default — correct pattern for RequestObserver.
  • The TEST_ONLY_ helper is properly gated behind experimental compat flag.
  • No backward compatibility concerns — purely additive changes (new virtual with default, new JSG method).
  • No security concerns — the new method just sets a metric flag, no capability leak.
  • No tests are added with this PR, but the TEST_ONLY_THROW_PYTHON_WORKERS_INTERNAL_ERROR helper suggests tests will be added separately (likely in the internal/production test suite).

The code is clean, follows existing patterns, and is purely additive. There are no actionable issues.

LGTM

github run

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 17, 2026

The generated output of @cloudflare/workers-types matches the snapshot in types/generated-snapshot 🎉

@dom96 dom96 force-pushed the dominik/python-hmd-metrics branch 3 times, most recently from cf1b851 to 132aa7a Compare April 20, 2026 11:07
@dom96 dom96 requested review from hoodmane and ryanking13 April 20, 2026 11:18
@dom96 dom96 marked this pull request as ready for review April 20, 2026 11:18
@dom96 dom96 requested review from a team as code owners April 20, 2026 11:18
@dom96 dom96 force-pushed the dominik/python-hmd-metrics branch from 132aa7a to 918ae4e Compare April 20, 2026 11:19
@dom96 dom96 force-pushed the dominik/python-hmd-metrics branch from 918ae4e to 789e44e Compare April 22, 2026 12:45
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Apr 22, 2026

Merging this PR will improve performance by 39.14%

⚡ 1 improved benchmark
✅ 71 untouched benchmarks
⏩ 129 skipped benchmarks1

Performance Changes

Benchmark BASE HEAD Efficiency
simpleStringBody[Response] 27.2 µs 19.5 µs +39.14%

Comparing dominik/python-hmd-metrics (789e44e) with main (764b4e8)

Open in CodSpeed

Footnotes

  1. 129 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@dom96 dom96 merged commit 5fdf8aa into main Apr 22, 2026
22 of 24 checks passed
@dom96 dom96 deleted the dominik/python-hmd-metrics branch April 22, 2026 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants