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

Don't throw on send-only endpoints #60

Open
timbussmann opened this issue Feb 18, 2019 · 7 comments
Open

Don't throw on send-only endpoints #60

timbussmann opened this issue Feb 18, 2019 · 7 comments

Comments

@timbussmann
Copy link
Contributor

Based on Particular/docs.particular.net#4264, is there a specific reason the metrics plugin throws at startup for send-only endpoints?

I understand that as of now, the captured metrics aren't that interesting for send-only endpoints however, throwing exceptions seems to be an unexpected and unusual behavior at fist glance.

@mikeminutillo
Copy link
Member

This is actually the root Metrics Plugin that does this. It was assumed to be a mistake to turn Metrics on for an endpoint that literally will report no metrics. If there's a good business reason to have this switched on then we can change it pretty easily.

@timbussmann
Copy link
Contributor Author

It was assumed to be a mistake to turn Metrics on for an endpoint that literally will report no metrics.

I'm not sure this is safe assumption since there is a good chance that users will reuse their configuration across endpoints, enabling monitoring by default. I think it would be better to not crash the endpoint but rather log a warning instead.

@mikeminutillo
Copy link
Member

@Particular/metrics-maintainers thoughts on this?

And can you move it to the Metrics Plugin repo?

@dvdstelt
Copy link
Member

I'm not sure this is safe assumption since there is a good chance that users will reuse their configuration across endpoints, enabling monitoring by default. I think it would be better to not crash the endpoint but rather log a warning instead.

Which users will not read and then contact us why they're not getting metrics on their sendonly endpoint.
If they reuse config across endpoints, they should make it so that they can alter the config before the endpoint is initialized/started.
If I can remember correctly this was thought through and the decision was to throw, to clearly notify users that this is not possible and at the same time reduce the number of support tickets.

@timbussmann
Copy link
Contributor Author

still feels that behavior is too much driven by the actual metrics currently available. What if we add a metric which makes sense on send-only endpoints?
How many other features which don't make sense in a specific context currently throw an exception at startup?

@tmasternak
Copy link
Member

tmasternak commented Mar 1, 2019

I also think that we should consider the principle of least surprise. With the current feature set if we enabled monitoring for send-only endpoints we would most likely have to add some explanation in SP to indicate that there are no metrics for send-only endpoints and this is fine.

In other words, enabling metrics for send-only endpoints is not only a plugin change.

@Scooletz
Copy link
Contributor

Scooletz commented Mar 4, 2019

@tmasternak Do you mean something like showing an information "this is a send-only endpoint" or something similar?

I do agree, that it's not only a plugin change.

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

No branches or pull requests

5 participants