Unifying c2s metrics #3967
Unifying c2s metrics #3967
Conversation
…obal.data.xmpp.message.processing_time
This comment was marked as outdated.
This comment was marked as outdated.
Codecov ReportBase: 83.46% // Head: 83.55% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #3967 +/- ##
==========================================
+ Coverage 83.46% 83.55% +0.08%
==========================================
Files 538 538
Lines 34003 34001 -2
==========================================
+ Hits 28382 28409 +27
+ Misses 5621 5592 -29
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
I'm generally very happy with all the changes, the only thing is that some are kinda breaking ones so they need documentation and potentially migration, which is something @chrzaszcz is working on. Dunno how do you want to proceed with that but apart from docs, this PR is exactly what I wanted, thank you so much 🎉
I was going to add documentation in the upcoming PR, when all the metrics are added (incl. s2s and xmpp components connections) |
| @@ -456,7 +456,7 @@ configure_mod_vcard(Config) -> | |||
| ldap_opts() -> | |||
| LDAPOpts = #{filter => <<"(objectClass=inetOrgPerson)">>, | |||
| base => <<"ou=Users,dc=esl,dc=com">>, | |||
| search_fields => [{"Full Name", "cn"}, {"User", "uid"}], | |||
| search_fields => [{<<"Full Name">>, <<"cn">>}, {<<"User">>, <<"uid">>}], | |||
| vcard_map => [{"FN", "%s", ["cn"]}]}, | |||
There was a problem hiding this comment.
Just a note: these should be binaries too.
| @@ -163,10 +163,6 @@ Metrics specific to an extension, e.g. Message Archive Management, are described | |||
| | ----------- | ---- | ----------- | | |||
| | `[global, data, xmpp, received, xml_stanza_size]` | histogram | A size (in bytes) of a received stanza after decompression and decryption. | | |||
| | `[global, data, xmpp, sent, xml_stanza_size]` | histogram | A size (in bytes) of a stanza sent to a client socket. | | |||
| | `[global, data, xmpp, received, compressed_size]` | histogram | A size (in bytes) of a received stanza before decompression. | | |||
There was a problem hiding this comment.
Maybe describe all new metrics?
| @@ -34,6 +34,18 @@ | |||
| modPrivacyStanzaAll | |||
| ]). | |||
|
|
|||
| -define(GLOBAL_SPIRALS, [ | |||
| %% TBD report raw data metrics for s2s and components conection | |||
| [data, xmpp, received, c2s, tcp, raw], | |||
There was a problem hiding this comment.
Do we have any plans for other suffixes than raw? It seems that it does not add much value since it is always the same.
There was a problem hiding this comment.
We agreed with @DenysGonchar that the comments will be addressed in a separate PR.
this PR introduces the following changes:
global.data.xmpp.*.compressed_sizemetrics - compression support is removed a while ago.global.data.xmpp.*.encrypted_sizemetrics - the size of a stanza doesn't change much after encryption, more over, when encryption is done for a batch of stanza then this metric is completely irrelevant.global.data.xmpp.*.xml_stanza_sizereporting - this histogram metrics make sense only when reported for a single stanza, this PR ensures that.global.data.xmpp.*.c2s*.rawspiral metrics for TCP/TLS/BOSH/WebSockets - firstly spiral calculation makes much more sense than histogram in addition to that it's less expensive, in the upcoming PR support for S2S and XMPP components traffic calculation will be added.mongoose_transportfor bosh socket - that's not needed any more, the same should be done for websockets in the following PR (as well as support of websockets connection for XMPP components)