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

Replace dots in "host" part of metric name with underscores. #2025

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kzemek
Copy link
Contributor

@kzemek kzemek commented Aug 8, 2018

Underscores are not a valid part of a domain name, so they
are safe to replace dots to in the metric name (i.e. this can't lead
to a collision with an existing host name).

Replacing dots with underscores prevents hosts from being treated
as multiple separate metrics nodes e.g. by Graphite and Grafana.

Underscores are not a valid part of a domain name, so they
are safe to replace dots to in the metric name (i.e. this can't lead
to a collision with an existing host name).

Replacing dots with underscores prevents hosts from being treated
as multiple separate metrics nodes e.g. by Graphite and Grafana.
@mongoose-im
Copy link
Collaborator

mongoose-im commented Aug 8, 2018

5360.1 / Erlang 19.3 / small_tests / c09f1a8
Reports root / small


5360.3 / Erlang 19.3 / mysql_redis / c09f1a8
Reports root/ big
OK: 2821 / Failed: 0 / User-skipped: 222 / Auto-skipped: 0


5360.2 / Erlang 19.3 / internal_mnesia / c09f1a8
Reports root/ big
OK: 1067 / Failed: 0 / User-skipped: 45 / Auto-skipped: 0


5360.6 / Erlang 19.3 / elasticsearch_and_cassandra_mnesia / c09f1a8
Reports root/ big
OK: 445 / Failed: 0 / User-skipped: 8 / Auto-skipped: 0


5360.5 / Erlang 19.3 / ldap_mnesia / c09f1a8
Reports root/ big
OK: 1031 / Failed: 0 / User-skipped: 81 / Auto-skipped: 0


5360.4 / Erlang 19.3 / odbc_mssql_mnesia / c09f1a8
Reports root/ big
OK: 2835 / Failed: 0 / User-skipped: 208 / Auto-skipped: 0


5360.8 / Erlang 20.0 / pgsql_mnesia / c09f1a8
Reports root/ big / small
OK: 2867 / Failed: 0 / User-skipped: 176 / Auto-skipped: 0


5360.9 / Erlang 21.0 / riak_mnesia / c09f1a8
Reports root/ big / small
OK: 1290 / Failed: 0 / User-skipped: 43 / Auto-skipped: 0

@codecov
Copy link

codecov bot commented Aug 8, 2018

Codecov Report

Merging #2025 into master will increase coverage by 0.09%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2025      +/-   ##
==========================================
+ Coverage   74.92%   75.02%   +0.09%     
==========================================
  Files         314      314              
  Lines       28619    28621       +2     
==========================================
+ Hits        21444    21472      +28     
+ Misses       7175     7149      -26     
Impacted Files Coverage Δ
src/mongoose_metrics.erl 92.59% <100.00%> (+0.11%) ⬆️
..._distrib/mod_global_distrib_outgoing_conns_sup.erl 73.91% <0.00%> (-8.70%) ⬇️
...rc/global_distrib/mod_global_distrib_transport.erl 47.05% <0.00%> (-5.89%) ⬇️
src/mod_muc_log.erl 77.69% <0.00%> (ø)
src/pubsub/mod_pubsub.erl 66.18% <0.00%> (+0.18%) ⬆️
src/ejabberd_c2s.erl 84.88% <0.00%> (+0.36%) ⬆️
src/mod_muc_room.erl 77.24% <0.00%> (+0.76%) ⬆️
src/mod_bosh_socket.erl 81.66% <0.00%> (+1.00%) ⬆️
src/global_distrib/mod_global_distrib_utils.erl 80.80% <0.00%> (+1.01%) ⬆️
src/muc_light/mod_muc_light_utils.erl 85.26% <0.00%> (+1.05%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f9029a1...fdf98fd. Read the comment docs.

@kzemek kzemek closed this Aug 8, 2018
@kzemek kzemek reopened this Aug 8, 2018
@mongoose-im
Copy link
Collaborator

mongoose-im commented Aug 8, 2018

5361.1 / Erlang 19.3 / small_tests / da2b9dd
Reports root / small


5361.3 / Erlang 19.3 / mysql_redis / da2b9dd
Reports root/ big
OK: 2821 / Failed: 0 / User-skipped: 222 / Auto-skipped: 0


5361.2 / Erlang 19.3 / internal_mnesia / da2b9dd
Reports root/ big
OK: 1067 / Failed: 0 / User-skipped: 45 / Auto-skipped: 0


5361.4 / Erlang 19.3 / odbc_mssql_mnesia / da2b9dd
Reports root/ big
OK: 2835 / Failed: 0 / User-skipped: 208 / Auto-skipped: 0


5361.5 / Erlang 19.3 / ldap_mnesia / da2b9dd
Reports root/ big
OK: 1031 / Failed: 0 / User-skipped: 81 / Auto-skipped: 0


5361.6 / Erlang 19.3 / elasticsearch_and_cassandra_mnesia / da2b9dd
Reports root/ big
OK: 445 / Failed: 0 / User-skipped: 8 / Auto-skipped: 0


5361.8 / Erlang 20.0 / pgsql_mnesia / da2b9dd
Reports root/ big / small
OK: 2867 / Failed: 0 / User-skipped: 176 / Auto-skipped: 0


5361.9 / Erlang 21.0 / riak_mnesia / da2b9dd
Reports root/ big / small
OK: 1290 / Failed: 0 / User-skipped: 43 / Auto-skipped: 0


5361.9 / Erlang 21.0 / riak_mnesia / da2b9dd
Reports root/ big / small
OK: 1290 / Failed: 0 / User-skipped: 43 / Auto-skipped: 0

Copy link
Member

@fenek fenek left a comment

Choose a reason for hiding this comment

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

  1. It should be configurable for now with a clear deprecation warning (for domains with dots) as it breaks compatibility with existing deployments.
  2. Mongoose-metrics.md update is necessary. Possibly there are more sections in documentation that should be changes as well.

@Neustradamus

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants