-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[paymentservice] add basic metrics support #583
[paymentservice] add basic metrics support #583
Conversation
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.
@pichlermarc would you mind updating our metric feature matrix too? Lgtm otherwise. Thanks for this! |
Oh and we'll need to update the payment service doc in the docs/services folder |
@cartersocha changed some text to the payment service docs to reflect the new setup, added some text for metrics, and changed the entry in the matrix 🙂 |
Thanks again @pichlermarc! Merging now |
* [paymentservice] add basic metrics support * add changelog add changelog * update docs * fix typos. * fix linting errors * clarify 'wrapper' approach Co-authored-by: Carter Socha <[email protected]>
Related to #68
Changes
This PR adds a counter
app.payment.transactions
that can be split by the transaction currency, adds dependencies to the JS Metrics packages, and changes the startup process so that the async resource detection can finish before creating aMeter
orTracer
.Once updated instrumentation packages are released, I will open a follow-up PR that updates the metrics API/SDK to the GA versions. Then the added temporality environment variable will also change the temporality of the exported metrics accordingly. 🙂