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

fix: change prefix format and pass New Relic sink #1

Closed

Conversation

peter-rr
Copy link
Collaborator

@peter-rr peter-rr commented Nov 16, 2023

Change prefix format for metric name and pass New Relic specific sink when creating a new recorder instance.

Relates to asyncapi#841

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@peter-rr peter-rr changed the title Change prefix format and pass New Relic sink fix: change prefix format and pass New Relic sink Nov 16, 2023
Copy link
Owner

@smoya smoya left a comment

Choose a reason for hiding this comment

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

There is one thing. You can't leave the NewRelicSink there since tests will try to send metrics over it. We should find the way to, in tests, just print metrics out over stdout or discard them.

Copy link
Owner

@smoya smoya left a comment

Choose a reason for hiding this comment

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

There is one thing. You can't leave the NewRelicSink there since tests will try to send metrics over it. We should find the way to, in tests, just print metrics out over stdout or discard them.

@peter-rr
Copy link
Collaborator Author

There is one thing. You can't leave the NewRelicSink there since tests will try to send metrics over it. We should find the way to, in tests, just print metrics out over stdout or discard them.

Should we then go back to the StdOutSink and only use NewRelicSink for our own tests until we find a better solution?

@smoya
Copy link
Owner

smoya commented Nov 20, 2023

There is one thing. You can't leave the NewRelicSink there since tests will try to send metrics over it. We should find the way to, in tests, just print metrics out over stdout or discard them.

Should we then go back to the StdOutSink and only use NewRelicSink for our own tests until we find a better solution?

Yes, I do. BTW, any idea on how to make that possible?

@peter-rr
Copy link
Collaborator Author

BTW, any idea on how to make that possible?

I guess we could print a brief message over stdout with the metrics sent, just to test we are actually sending them regardless the sink we're using. And then only running the tests containing all the information about the metrics when executing the tests for an specific sink like New Relic. Does it make sense for you?

@peter-rr
Copy link
Collaborator Author

BTW, any idea on how to make that possible?

Ok, I see you've already solved it here 👍

@smoya smoya force-pushed the feat/adoptionMetrics branch from c9027b5 to e9e79f6 Compare November 22, 2023 21:36
@smoya
Copy link
Owner

smoya commented Nov 23, 2023

Closing in favor of #2

@smoya smoya closed this Nov 23, 2023
@peter-rr peter-rr deleted the feat/adoptionMetrics branch February 2, 2024 10:28
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