-
Notifications
You must be signed in to change notification settings - Fork 18
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
Metrics refactor #221
Metrics refactor #221
Conversation
…on of those interfaces for Opentelemetry Added a MetricsEngine that handles creation and registration of metrics
…lication, storage and API metrics Added the MetricsEngine to the service context
…ld singleton metrics instance
Added a MetricsHelper to aid with test setup for metrics
# Conflicts: # modules/module-mongodb/src/replication/ChangeStream.ts # modules/module-mongodb/test/src/change_stream_utils.ts # modules/module-postgres/test/src/slow_tests.test.ts # packages/service-core-tests/src/tests/register-sync-tests.ts
🦋 Changeset detectedLatest commit: 7c358b8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
# Conflicts: # modules/module-mongodb/test/src/change_stream_utils.ts # modules/module-mysql/test/src/BinlogStreamUtils.ts # modules/module-postgres/test/src/wal_stream_utils.ts # packages/service-core-tests/src/tests/register-sync-tests.ts # packages/service-core/src/routes/endpoints/socket-route.ts # packages/service-core/src/routes/endpoints/sync-stream.ts # packages/service-core/src/system/ServiceContext.ts
# Conflicts: # modules/module-postgres/src/replication/WalStream.ts
# Conflicts: # packages/service-core-tests/src/tests/register-sync-tests.ts
Consolidated some extra logging info from powersync cloud runners into service runners
I tested this on the hosted powersync on staging, and did not pick up any issues. Metrics are still being correctly reported to Prometheus. |
… it is started in a replication context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
skimmed, but happy with the patterns and architecture used. Nice work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some minor comments. Overall this looks brilliant.
…ed powersync config
7c358b8
Refactored metrics to use a MetricsEngine which forms part of the service context.
Metrics and their creation are abstracted away behind interfaces decoupled from Opentelemetry.
Metrics can be created and accessed via the MetricsEngine.
Moved the metrics initialisation code out of the service and into service core. That way the initialisation code can be re-used by the powersync cloud edition.