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.

Sign up for GitHub

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

Jump to bottom

DM-46821: Refactor metrics configuration #311

Merged
merged 6 commits into from
Oct 18, 2024
Merged

DM-46821: Refactor metrics configuration #311

merged 6 commits into from
Oct 18, 2024

Commits on Oct 18, 2024

  1. Refactor metrics configuration 

    Rework the metrics configuration so that `MetricsConfiguration` is
a union type supporting either a disabled or a Kafka-based metrics
configuration, which can be used as a drop-in type in an application
`BaseSettings` class and will automatically select one or the other
based on what configuration is available. Have those two configuration
classes in turn create two different `EventManager` classes that
provide the same interface, and two different underlying `EventPublisher`
classes. Use this to implement the no-op metrics functionality rather
than checking a boolean. This allows all of the Kafka configuration
to be omitted if metrics are disabled.

Update the documentation accordingly, and fix a few minor problems
with the documentation discovered as part of the Gafaelfawr integration.

Move the `app_name` field to the top level of the metrics configuration
on the grounds that this will likely apply to any application metrics,
including things that are not events. Rename `metrics_events` to just
`events`, since the metrics part is clear in context.

Use the `topic_prefix` name uniformly instead of `base_topic_prefix`.
    @rra
    rra committed Oct 18, 2024
    Configuration menu
    Copy the full SHA
    ae9134c View commit details
    Browse the repository at this point in the history

  2. Rename app_name to application in metrics 

    `application` looks a bit nicer in YAML configuration since it
doesn't require handling camel-case transformations.
    @rra
    rra committed Oct 18, 2024
    Configuration menu
    Copy the full SHA
    2fdec5b View commit details
    Browse the repository at this point in the history

  3. Ignore the json_encoders deprecation warning 

    We will get rid of this code as soon as there is a better solution
in dataclasses-avroschema.
    @rra
    rra committed Oct 18, 2024
    Configuration menu
    Copy the full SHA
    a35a5f4 View commit details
    Browse the repository at this point in the history

  4. Improve docstring for topic name 

    Be clearer about what the name passed to `build_validated_model`
in the `EventManager` class is used for.
    @rra
    rra committed Oct 18, 2024
    Configuration menu
    Copy the full SHA
    c29b984 View commit details
    Browse the repository at this point in the history

  5. Refactor safir.metrics.EventManager 

    Refactor the `EventManager` classes to share more structural code
with subclasses. The duplicate detection and iniitalization detection
code has moved to the parent class, and subclasses now only provide
the code to create a publisher from an enriched model, and can
override initialize and aclose if needed.
    @rra
    rra committed Oct 18, 2024
    Configuration menu
    Copy the full SHA
    b0bf28d View commit details
    Browse the repository at this point in the history

  6. Move event manager logger to parent class 

    The logic for event manager loggers is common to all of the
implementations, so move it into the parent class.
    @rra
    rra committed Oct 18, 2024
    Configuration menu
    Copy the full SHA
    116ee41 View commit details
    Browse the repository at this point in the history