feature(1026): add sessioncount to prometheus metrics #1075
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
paulkram
force-pushed
the
feature/paulkra/add-more-prometheus-metrics
branch
from
c126e4e to
9f27e06
Compare
paulkram
commented
Nov 3, 2022
| void prom_inc_allocation(void); | ||
| void prom_dec_allocation(void); | ||
| void prom_inc_allocation(SOCKET_TYPE type); | ||
| void prom_dec_allocation(SOCKET_TYPE type); | ||
| #else | ||
|
|
||
| void start_prometheus_server(void); |
There was a problem hiding this comment.
I think this method does not need to be guarded by the TURN_NO_PROMETHEUS define.
There was a problem hiding this comment.
I agree - we should have minimal amount if ifdefs outside of prom_server.c file to improve readability of the code
eakraly
reviewed
Nov 3, 2022
eakraly
reviewed
Nov 3, 2022
eakraly
reviewed
Nov 3, 2022
| @@ -201,7 +201,7 @@ int ur_map_foreach(ur_map* map, foreachcb_type func) { | |||
| return 0; | |||
| } | |||
|
|
|||
| int ur_map_foreach_arg(ur_map* map, foreachcb_arg_type func, void* arg) { | |||
| int ur_map_foreach_arg(const ur_map* map, foreachcb_arg_type func, void* arg) { | |||
There was a problem hiding this comment.
Do we want this in this PR? What does it fix?
There was a problem hiding this comment.
I had another approach how to add more metrics at first and missed to remove that. This only improves readability for new devs to understand which are in and which are out parameters.
|
Hi Pavel,
I saw at least "udp" and "tcp" during my tests but I ran it with some clients and more in a chaotic scenario. We should monitor the values (I'll do) and if there is something broken, I'm going to open another PR.
Gesendet: Sonntag, 06. November 2022 um 19:16 Uhr
Von: "Pavel Punsky" ***@***.***>
An: "coturn/coturn" ***@***.***>
Cc: "Paul Kramer" ***@***.***>, "Mention" ***@***.***>
Betreff: Re: [coturn/coturn] feature(1026): add sessioncount to prometheus metrics (PR #1075)
Hi @paulkram !
I checked out your PR and found one more issue: I always get socket type UDP in prometheus metric - not sure if this is me testing it incorrectly or it is broken somehow (I tried with -t -T)
Also, as I suspected, "ALL" and "UDP" are duplicates
Thanks!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
This was referenced Dec 4, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.