Fix typespec for drop option

[jaminthorns]

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

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

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

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

Additional details and impacted files

@@             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	Coverage Δ
lib/telemetry_metrics.ex	98.95% <ø> (-1.05%)	⬇️

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b9675ff...096dbf3. Read the comment docs.

[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]

💚 💙 💜 💛 ❤️