search

akoutmos / prom_ex

An Elixir Prometheus metrics collection library built on top of Telemetry with accompanying Grafana dashboards
MIT License
577 stars 96 forks source link

[BUG] Correct Supervisor Setup Documentation For PromEx #211

Open conradwt opened 10 months ago

conradwt commented 10 months ago

Describe the bug

When running mix prom_ex.gen.config --datasource "Local Prometheus", the generated docs says the following in step 2:

Add this module to your application supervision tree. It should be one of the first
     things that is started so that no Telemetry events are missed. For example, if PromEx
     is started after your Repo module, you will miss Ecto's init events and the dashboards
     will be missing some data points

However, the README.md for the project mentions the following:

PromEx should be started after the Endpoint, to avoid unnecessary error messages

The supervised children for the application look like the following:

    children = [
      # Start the PromEx
      ZeroPhoenix.PromEx,
      # Start the Telemetry supervisor
      ZeroPhoenixWeb.Telemetry,
      # Start the Ecto repository
      ZeroPhoenix.Repo,
      # Start the PubSub system
      {Phoenix.PubSub, name: ZeroPhoenix.PubSub},
      # Start Finch
      {Finch, name: ZeroPhoenix.Finch},
      # Start the Endpoint (http/https)
      ZeroPhoenixWeb.Endpoint
    ]

To Reproduce Steps to reproduce the behavior:

  1. mix prom_ex.gen.config --datasource "Local Prometheus"
  2. Review prom_ex.ex and project README.md

Expected behavior

I would expect the generated code docs (i.e. prom_ex.ex) and the README.md would be consistent here.

Environment

Additional context

Just looking for better clarification within the documentation as to the proper location for the PromEx within the supervision tree.

tsantero commented 5 months ago

I came here seeking the same clarification to the documentation inconsistency raised by @conradwt in this unaddressed issue.

In my experience I'm inclined to start the prom_ex supervision tree before all other applications for the exact reason mentioned in the README and quoted by OP.

If any project authors have the time to chime in here it would be greatly appreciated. ❤️