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

Built-in Metrics support #65

Closed
krsna1729 opened this issue Jan 20, 2020 · 2 comments
Closed

Built-in Metrics support #65

krsna1729 opened this issue Jan 20, 2020 · 2 comments

Comments

@krsna1729
Copy link
Contributor

Wondering if metrics #64 should be added to conn/sess itself? Of course it should be optional.

annotated with type/raddr/TEID/bearer/IMSI/TAI labels where possible

  1. Counters for message received/sent (already done in GW Tester: Add Prometheus support #64 with type/raddr labels)
  2. Histogram for time spent in system
@wmnsk
Copy link
Owner

wmnsk commented Jan 20, 2020

Thank you for your idea! 😃
I'd like to consider that carefully from these two points;

  1. I don't think it's not so common to implement Prometheus directly into the package itself (not the app). It might not be happy to have an additional dependency for those who are not interested in that.
  2. Even if we decide to implement it, labeling with TEID/bearer/IMSI/TAI is not a good practice according to the Prometheus doc, as it can easily be excessive from the cardinality point of view.
    https://prometheus.io/docs/practices/instrumentation/#do-not-overuse-labels

So, my opinion is just to add the methods that are helpful for instrumentation(like (*Conn).SessionCount() which is already available). Please tell me what you think.

@krsna1729
Copy link
Contributor Author

Yes makes sense to give all the methods that will be helpful to gather the necessary information to annotate or measure.

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

2 participants