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

Use name instead of event_name in metric uniqueness in Reporter #10

Closed
wants to merge 1 commit into from

Conversation

MathieuLegault1
Copy link

Instead of using the event_name as a unique_identifier when determining when event is unique, we change it so that it now uses the name. The change is needed since in certain case we want to measure the same event_name in multiple ways. This happens when using the keep or old function to discard some events. For example, in the telemetry_metrics hexdoc we have this:

distribution(
  "http.client.request.duration",
  event_name: [:http_client, :request, :stop],
  drop: &(match?(%{name: :fast_client}, &1))
)

distribution(
  "http.fast.client.request.duration",
  event_name: [:http_client, :request, :stop],
  keep: &(match?(%{name: :fast_client}, &1))
)

As we can see, the same event_name is used twice which would be merged together at the moment. Since we need to log them individually, we need to use the name instead of event_name

Instead of using the `event_name` as a unique_identifier when determining when event is unique, we change it so that it now uses the `name`. The change is needed since in certain case we want to measure the same event_name in multiple ways. This happens when using the keep or old function to discard some events. For example, in the telemetry_metrics hexdoc we have this

```elixir
distribution(
  "http.client.request.duration",
  event_name: [:http_client, :request, :stop],
  drop: &(match?(%{name: :fast_client}, &1))
)

distribution(
  "http.fast.client.request.duration",
  event_name: [:http_client, :request, :stop],
  keep: &(match?(%{name: :fast_client}, &1))
)
```

As we can see, the same event_name is used twice which would be merged together at the moment. Since we need to log them individually, we need to use the `name` instead of `event_name`
@simonprev
Copy link
Member

Since this is a breaking change, I just released version 2.0 that address this issue 🎉

I also added instructions in the README: https://github.com/mirego/telemetry_ui#20-breaking-change

@simonprev simonprev closed this Feb 6, 2023
@simonprev simonprev deleted the fix/uniq_metric_by_event_name branch February 6, 2023 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants