Instrument/label consistency #4228
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/instrument #4228 +/- ##
===================================================
Coverage 84.39% 84.40%
===================================================
Files 557 556 -1
Lines 33588 33630 +42
===================================================
+ Hits 28348 28386 +38
- Misses 5240 5244 +4 ☔ View full report in Codecov by Sentry. |
chrzaszcz
force-pushed
the
instrument/label-consistency
branch
from
4cf4a30 to
a260d18
Compare
This comment was marked as outdated.
This comment was marked as outdated.
chrzaszcz
force-pushed
the
instrument/label-consistency
branch
from
a260d18 to
78074d5
Compare
This comment was marked as outdated.
This comment was marked as outdated.
chrzaszcz
force-pushed
the
instrument/label-consistency
branch
from
78074d5 to
7436e6d
Compare
This comment was marked as outdated.
This comment was marked as outdated.
chrzaszcz
force-pushed
the
instrument/label-consistency
branch
from
7436e6d to
718f018
Compare
This comment was marked as outdated.
This comment was marked as outdated.
chrzaszcz
force-pushed
the
instrument/label-consistency
branch
from
718f018 to
050a842
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
chrzaszcz
force-pushed
the
instrument/label-consistency
branch
from
a217513 to
3469900
Compare
This comment was marked as outdated.
This comment was marked as outdated.
chrzaszcz
force-pushed
the
instrument/label-consistency
branch
from
3469900 to
582da60
Compare
This comment was marked as outdated.
This comment was marked as outdated.
The state is kept in a gen_server, and after calling persist/0 it becomes mirrored in a persistent term, allowing efficient lookup. Main benefits: - Consistency of setup/teardown - previously these operations might intialize metrics, but fail to register handlers (e.g. for duplicates) - Ability to prevent label key inconsistency for an event, which would make prometheus fail. - Similarity to gen_hook - Maybe a slight advantage in terms of efficiency (persistent term vs ETS) - Less possibility of race conditions, because all modifications are sequential. Potential bottlenecks: - Before calling persist(), all operations are handled by the gen_server, but no load should arrive yet (hooks are inactive). - After calling persist(), modifications are more costly, because the persistent term will be updated for each of them. This shouldn't be a problem, because at that point all instrumentation is already set up.
- Test functionality with and without the persistent term. - Check label consistency, which is now enforced. - Change the way call history is gathered - now the functions are executed in a different process, so filter by event name.
chrzaszcz
force-pushed
the
instrument/label-consistency
branch
from
582da60 to
277171b
Compare
|
elasticsearch_and_cassandra_26 / elasticsearch_and_cassandra_mnesia / 277171b small_tests_25 / small_tests / 277171b small_tests_26 / small_tests / 277171b small_tests_26_arm64 / small_tests / 277171b ldap_mnesia_26 / ldap_mnesia / 277171b dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 277171b dynamic_domains_mysql_redis_26 / mysql_redis / 277171b ldap_mnesia_25 / ldap_mnesia / 277171b dynamic_domains_pgsql_mnesia_26 / pgsql_mnesia / 277171b internal_mnesia_26 / internal_mnesia / 277171b pgsql_cets_26 / pgsql_cets / 277171b dynamic_domains_mssql_mnesia_26 / odbc_mssql_mnesia / 277171b pgsql_mnesia_25 / pgsql_mnesia / 277171b mysql_redis_26 / mysql_redis / 277171b pgsql_mnesia_26 / pgsql_mnesia / 277171b mssql_mnesia_26 / odbc_mssql_mnesia / 277171b |
Reimplement
mongoose_instrumentregistry, allowing more consistency checks.EventNameneed to be the same for all registered events - otherwise Prometheus metrics would break.set_upcallbacks are not called before raising the error.A
gen_serveris used to eliminate possible race conditions during setup.Previously,
mongoose_instrument_registrywas an ETS table. Now a map:#{event_name() => #{labels() => handlers()}}is used instead.gen_server, but a call tomongoose_instrument:persist()should be done after initial setup, which stores it in a persistent term.set_up/tear_downcall.persistit is optimized for concurrent reads.This is very similar to
gen_hook, but the events are handled correctly even beforepersistis called. This seems to be needed, because some events might be triggered during startup, and they shouldn't fail.Note: after a restart the persistent terms are always deleted, even if
terminatewas not called (e.g.kill). This means that all instrumentation is lost on restart. We could change this to make it permanent, but I think it would require a separate PR. The same applies togen_hook, but additionally it can get into an inconsistent state when killed and started again - this is unlikely though.