[idea] Amount of magic for publishing metrics

[allada]

Looking for feedback on how automatical publishing metrics should be?

Right now I have 2 approaches, a procedural, but verbose one and a magical macro one.

Here's the syntax for the two:

// FilesystemStore example.
impl<Fe: FileEntry> MetricsComponent for FilesystemStore<Fe> {
    fn gather_metrics(&self, c: &mut CollectorState) {
        c.publish(
            "read_buff_size",
            self.read_buffer_size,
            "Size of the configured read buffer size",
        );
        c.publish(
            "active_drop_spawns",
            &self.shared_context.active_drop_spawns,
            "Number of active drop spawns",
        );
        c.publish(
            "temp_path",
            &self.shared_context.temp_path,
            "Path to the configured temp path",
        );
        c.publish(
            "content_path",
            &self.shared_context.content_path,
            "Path to the configured content path",
        );
        c.publish("evicting_map", &self.evicting_map, "");
    }
}

// VerifyStore example.
impl MetricsComponent for VerifyStore {
    fn gather_metrics(&self, c: &mut CollectorState) {
        c.publish(
            "verify_size",
            self.verify_size,
            "If the verification store is verifying the size of the data",
        );
        c.publish(
            "verify_hash",
            self.verify_hash,
            "If the verification store is verifying the hash of the data",
        );
        c.publish(
            "size_verification_failures",
            &self.size_verification_failures,
            "Number of failures the verification store had due to size mismatches",
        );
        c.publish(
            "hash_verification_failures",
            &self.hash_verification_failures,
            "Number of failures the verification store had due to hash mismatches",
        );
    }
}

// MemoryStore example.
impl MetricsComponent for MemoryStore {
    fn gather_metrics(&self, c: &mut CollectorState) {
        c.publish("evicting_map", &self.evicting_map, "");
    }
}

The macro version would look like this:

// FilesystemStore example.
publish_metrics! {
    FilesystemStore<Fe> {
        evicting_map,
        read_buff_size "Size of the configured read buffer size" Bytes,
        shared_context {
            active_drop_spawns "Number of active drop spawns",
            temp_path "Path to the configured temp path",
            content_path "Path to the configured content path",
        }
    }
}

// VerifyStore example.
publish_metrics! {
    VerifyStore {
        verify_size "If the verification store is verifying the size of the data",
        verify_hash "If the verification store is verifying the hash of the data",
        size_verification_failures "Number of failures the verification store had due to size mismatches",
        hash_verification_failures "Number of failures the verification store had due to hash mismatches",
    }
}

// MemoryStore example.
publish_metrics! {
    MemoryStore {
        evicting_map,
    }
}

These two examples would do 100% identical things, except the macro one would also make it easy to denote the type (which is a bit tricky to do with the procedural one.

Thoughts?

[allada]

cc: @chrisstaite-menlo, @aaronmondal

[chrisstaite-menlo]

Oh, I do love the macros...

[chrisstaite-menlo]

Although, it is going to be confusing because of the "magic" self... I think on second thoughts could you go half way...

impl<Fe: FileEntry> MetricsComponent for FilesystemStore<Fe> {
    fn gather_metrics(&self, c: &mut CollectorState) {
        publish_metrics!(
            c,
            {
                self.read_buffer_size "Size of the configured read buffer size",
                self.shared_context.active_drop_spawns "Number of active drop spawns"
            }
         );
      }
}

[allada]

If I was to go that far, I'd use something like:

impl<Fe: FileEntry> MetricsComponent for FilesystemStore<Fe> {
    fn gather_metrics(&self, c: &mut CollectorState) {
        publish!(c, read_buffer_size, "Size of the configured read buffer size");
        publish!(c, active_drop_spawns, "Number of active drop spawns");
      }
}

This would allow you to also do your custom stuff when needed.

[allada]

Keep in mind though... This won't be a normal macro, this will require a procedural macro, which is way more difficult to understand, because it rewrites the AST tree.

I also considered something like this:

#[metrics]
pub struct SharedContext {
    // Used in testing to know how many active drop() spawns are running.
    // TODO(allada) It is probably a good idea to use a spin lock during
    // destruction of the store to ensure that all files are actually
    // deleted (similar to how it is done in tests).
    #[metric("Number of active drop spawns")]
    pub active_drop_spawns: AtomicU64,

    #[metric("Path to the configured temp path")]
    temp_path: String,

    #[metric("Path to the configured content path")]
    content_path: String,
}

#[metrics]
pub struct FilesystemStore<Fe: FileEntry = FileEntryImpl> {
    #[metric]
    evicting_map: EvictingMap<Arc<Fe>, SystemTime>,

    #[metric]
    shared_context: Arc<SharedContext>,

    #[metric("Size of the configured read buffer size")]
    read_buffer_size: usize,
}

I really like this idea, but it makes it incredibly difficult to do some edge case management.

[allada]

I just went down a rabbit hole that I need to get out of... I have so many ideas on what to do with metrics and now I want to write a metrics library.

[aaronmondal]

Hmm hard question 😅 I wonder how complex a procedural macro publish_metrics actually is. If it roughly rewrites Option 2 to something like Option 1, it might be on the simpler side where we could get away with more-or-less straightforward templating. Then again, that's basically the same thing as implementing the traits explicitly.

From another angle, could a custom serde serializer/deserializer impl be useful here? With the container attributes that might allow behavior somewhat similar to the #[metric] approach. https://serde.rs/impl-serialize.html