Fix typespec for `drop` option

[jaminthorns]

I forgot to update this in 3fde8304532670af7ebf57cbb041103be8a15774. Now we're using the same keep_fun type for both keep and drop to prevent them getting out-of-sync.

[codecov-commenter]

:warning: Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 98.65%. Comparing base (ab86164) to head (096dbf3). Report is 3 commits behind head on main.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

[](https://app.codecov.io/gh/beam-telemetry/telemetry_metrics/pull/114?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=beam-telemetry) ```diff @@ Coverage Diff @@ ## main #114 +/- ## =========================================== - Coverage 100.00% 98.65% -1.35% =========================================== Files 2 2 Lines 145 149 +4 =========================================== + Hits 145 147 +2 - Misses 0 2 +2 ``` | [Files with missing lines](https://app.codecov.io/gh/beam-telemetry/telemetry_metrics/pull/114?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=beam-telemetry) | Coverage Δ | | |---|---|---| | [lib/telemetry\_metrics.ex](https://app.codecov.io/gh/beam-telemetry/telemetry_metrics/pull/114?src=pr&el=tree&filepath=lib%2Ftelemetry_metrics.ex&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=beam-telemetry#diff-bGliL3RlbGVtZXRyeV9tZXRyaWNzLmV4) | `98.95% <ø> (-1.05%)` | :arrow_down: | ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/beam-telemetry/telemetry_metrics/pull/114?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=beam-telemetry). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=beam-telemetry) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/beam-telemetry/telemetry_metrics/pull/114?dropdown=coverage&src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=beam-telemetry). Last update [b9675ff...096dbf3](https://app.codecov.io/gh/beam-telemetry/telemetry_metrics/pull/114?dropdown=coverage&src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=beam-telemetry). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=beam-telemetry).

[bryannaegele]

I think it needs a new name entirely then as this feels confusing. I don't have a constructive opinion on what it should be. drop could still be kept and the type signature updated. That's probably the simplest route.

[josevalim]

Changing the type names can be a backwards incompatible change. So the easiest is to make it so @type drop :: keep.

[jaminthorns]

@josevalim Didn't realize that about backwards incompatibility, thank you!

@bryannaegele Do you think it would be confusing to implement @josevalim's suggestion (drop being an alias of keep)? I can instead just duplicate the the types like they were before:

  @type keep ::
          (:telemetry.event_metadata() -> boolean())
          | (:telemetry.event_metadata(), :telemetry.event_measurements() -> boolean())
  @type drop ::
          (:telemetry.event_metadata() -> boolean())
          | (:telemetry.event_metadata(), :telemetry.event_measurements() -> boolean())

But I consolidated them to prevent a mistake like the one I made, especially now that they're more complex. I have no issue with this if it's more readable, though.

[bryannaegele]

I think aliasing to keep would be confusing. Copying is the simplest. If I were forced to refactor it, I'd alias the values.

Something like

@type metadata_predicate_fun() :: (:telemetry.event_metadata() -> boolean())
@type metadata_measurement_predicate_fun() :: (:telemetry.event_metadata(), :telemetry.event_measurements() -> boolean())

@type keep :: metadata_predicate_fun() | metadata_measurement_predicate_fun()
@type drop :: metadata_predicate_fun() | metadata_measurement_predicate_fun()

[josevalim]

@type drop :: keep is the simplest IMO.

[bryannaegele]

It could be me but I imagine it would be confusing for a new user to see a signature saying drop is a keep lol.

[josevalim]

It is a type annotation. The type annotation should mean they both have the same type. It doesn't mean they have the same semantics. :)

[jaminthorns]

Just pushed up a change. I think the predicate_fun type strikes a balances between clarity (mirroring the "predicate function" language from the documentation) and simplicity (just 1 extra type). Let me know if you'd like any more changes!

[josevalim]

:green_heart: :blue_heart: :purple_heart: :yellow_heart: :heart: