Clarify filter.not() w.r.t. event_enabled

[CAD97]

No description provided.

[hawkw]

i'm not completely sure how i feel about the use of Rust-like pseudocode in the documentation; it seems like it could potentially create some confusion if people incorrectly interpret it actual code. do you think it would make sense to document this as a Markdown table or something, instead? i'm fine with this if it's really the best way to document this behavior, but i am mildly skeptical about it...

[CAD97]

The thing which is easier explained with pseudocode is the short circuiting behavior. Also, just trying to reason about using a filter on top of a layer which reports global filtering is overloading my brain atm, so...

That said, here's an attempt at a table to communicate the same information plus a bit:

Normal

F::callsite_enabled	F::enabled	S::enabled	F::event_enabled	S::event_enabled	lifecycle callbacks
ALWAYS	✗	✗	✗	✗	✓
NEVER	✗	✗	✗	✗	✗
SOMETIMES	true	✓	✗	✗	✓ (span)
SOMETIMES	true	✓	true	✓	✓ (event)
SOMETIMES	true	✓	false	✗	✗ (event)
SOMETIMES	false	✗	✗	✗	✗

Not

F::callsite_enabled	F::enabled	S::enabled	F::event_enabled	S::event_enabled	lifecycle callbacks
ALWAYS	✗	✗	✗	✗	✗
NEVER	✗	✗	✗	✗	✓
SOMETIMES	true	✗	✗	✗	✗
SOMETIMES	false	✓	✗	✗	✓ (span)
SOMETIMES	false	✓	✗	✓	✓ (event)

Pure logical filter inverse without time travel

F::callsite_enabled	F::enabled	S::enabled	F::event_enabled	S::event_enabled	lifecycle callbacks
ALWAYS	✗	✗	✗	✗	✗
NEVER	✗	✗	✗	✗	✓
SOMETIMES	true	✗	✗	✗	✗ (span)
SOMETIMES	true	✓	true	✗	✗ (event)
SOMETIMES	true	✓	false	✓	✓ (event)
SOMETIMES	false	✓	✗	✗	✓ (span)
SOMETIMES	false	✓	✗	✓	✓ (event)

---

I wish I could give a nice visual diff between the tables, but this'll have to do: diff Not to "logical":

 | `F::callsite_enabled` | `F::enabled` | `S::enabled` | `F::event_enabled` | `S::event_enabled` | lifecycle callbacks |
 | :-:                   | :-:          | :-:          | :-:                | :-:                | :-:                 |
 | `ALWAYS`              | ✗           | ✗           | ✗                 | ✗                 | ✗                  |
 | `NEVER`               | ✗           | ✗           | ✗                 | ✗                 | ✓                  |
-| `SOMETIMES`           | `true`       | ✗           | ✗                 | ✗                 | ✗                  |
+| `SOMETIMES`           | `true`       | ✗           | ✗                 | ✗                 | ✗ (span)           |
+| `SOMETIMES`           | `true`       | ✓           | `true`             | ✗                 | ✗ (event)          |
+| `SOMETIMES`           | `true`       | ✓           | `false`            | ✓                 | ✓ (event)          |
 | `SOMETIMES`           | `false`      | ✓           | ✗                 | ✗                 | ✓ (span)           |
 | `SOMETIMES`           | `false`      | ✓           | ✗                 | ✓                 | ✓ (event)          |

[hawkw]

hmm, yeah, that's...not great. i'm fine with the pseudocode instead, then.

[CAD97]

(It's worth noting that the pseudocode doesn't contain the S::* information from the tables, so for a fair comparison drop those two columns. But yeah, communicating short circuiting in a table is hard.)

[CAD97]

You mentioned thinking maybe we should've made event_enabled return Option<bool>; the "correct" behavior I think would've been for (a time machine and) enabled to also return Interest, then iff that returns a sometimes() calling span_enabled/event_enabled for a final determination.

So roughly

• Cached once per callsite, callsite_enabled.
• Each time span! is entered:
  • if callsite enabled always: Evaluate fields. Return active span.
  • if callsite enabled sometimes:
    • Check enabled.
    • if enabled always: Evaluate fields. Return active span.
    • if enabled sometimes:
      • Evaluate fields.
      • If span_enabled: Return active span.
  • Return disabled span.
• Each time event! is entered:
  • if callsite enabled always: Evaluate fields. Emit event.
  • if callsite enabled sometimes:
    • Check enabled.
    • If enabled always: Evaluate fields. Emit event.
    • If enabled sometimes:
      • Evaluate fields.
      • If event_enabled: Emit event.

In terms of signatures,

• Collect
  • register_callsite(&self, &Metadata) -> Interest
  • enabled(&self, &Metadata) -> Interest
  • span_enabled(&self, &Attributes) -> bool
  • event_enabled(&self, &Attributes) -> bool
• Subscribe (but only local filtering)
  • register_callsite(&self, &Metadata) -> Interest
  • enabled(&self, &Metadata, &Context) -> Interest
  • span_enabled(&self, &Attributes, &Context) -> bool
  • event_enabled(&self, &Attributes, &Context) -> bool
• Per-layer filtering is done by layering Subscribe in a tree fashion.

(I'd personally also say that the enabled functions not being called when !interest.is_sometimes() is purely an optimization and you are not allowed to rely on this being true. This isn't necessary, but it's imho easier to reason about for wrapping collectors/filters.)

(Yes, there's complexity in the system I'm not fully aware of that probably makes that too simple.)

[hawkw]

noticed a misspelling that got duplicated a bunch of times (presumably by copying and pasting it). besides that, looks good to me!

feedback addressed