refac(subscriber): make Callsites length const generic

console-subscriber/src/callsites.rs

@@ -1,20 +1,15 @@
 use std::{
-    ptr,
+    fmt, ptr,
     sync::atomic::{AtomicPtr, AtomicUsize, Ordering},
 };
 use tracing_core::Metadata;
 
-#[derive(Debug, Default)]
-pub(crate) struct Callsites {
+pub(crate) struct Callsites<const MAX_CALLSITES: usize> {
     ptrs: [AtomicPtr<Metadata<'static>>; MAX_CALLSITES],
     len: AtomicUsize,
 }
 
-// In practice each of these will have like, 1-5 callsites in it, max, so
-// 32 is probably fine...if it ever becomes not fine, we'll fix that.
-const MAX_CALLSITES: usize = 32;
-
-impl Callsites {
+impl<const MAX_CALLSITES: usize> Callsites<MAX_CALLSITES> {
     #[track_caller]
     pub(crate) fn insert(&self, callsite: &'static Metadata<'static>) {
         // The callsite may already have been inserted, if the callsite cache
@@ -27,9 +22,10 @@ impl Callsites {
         let idx = self.len.fetch_add(1, Ordering::AcqRel);
         assert!(
             idx < MAX_CALLSITES,
-            "you tried to store more than 64 callsites, \
+            "you tried to store more than {} callsites, \
             time to make the callsite sets bigger i guess \
-            (please open an issue for this)"
+            (please open an issue for this)",
+            MAX_CALLSITES,
         );
         self.ptrs[idx]
             .compare_exchange(
@@ -51,3 +47,39 @@ impl Callsites {
         false
     }
 }
+
+impl<const MAX_CALLSITES: usize> Default for Callsites<MAX_CALLSITES> {
+    fn default() -> Self {
+        // It's necessary to use a `const` value here to initialize the array,
+        // because `AtomicPtr` is not `Copy`.
+        //
+        // Clippy does not like when `const` values have interior mutability. See:
+        // https://rust-lang.github.io/rust-clippy/master/index.html#declare_interior_mutable_const
+        //
+        // This is a warning because the const value is always copied when it's
+        // used, so mutations to it will not be reflected in the `const` itself.
+        // In some cases, this is a footgun (when you meant to use a `static`
+        // item instead). However, in this case, that is *precisely* what we
+        // want; the `const` value is being used as an initializer for the array
+        // and it is *supposed* to be copied. Clippy's docs recommend ignoring
+        // the lint when used as a legacy const initializer for a static item;
+        // this is a very similar case.
+        #[allow(clippy::declare_interior_mutable_const)]
+        const NULLPTR: AtomicPtr<Metadata<'static>> = AtomicPtr::new(ptr::null_mut());
+        Self {
+            ptrs: [NULLPTR; MAX_CALLSITES],
+            len: AtomicUsize::new(0),
+        }
+    }
+}
+
+impl<const MAX_CALLSITES: usize> fmt::Debug for Callsites<MAX_CALLSITES> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let len = self.len.load(Ordering::Acquire);
+        f.debug_struct("Callsites")
+            .field("ptrs", &&self.ptrs[..len])
+            .field("len", &len)
+            .field("max_callsites", &MAX_CALLSITES)
+            .finish()
+    }
+}

console-subscriber/src/lib.rs

@@ -3,6 +3,7 @@ use serde::Serialize;
 use tokio::sync::{mpsc, oneshot};
 
 use std::{
+    fmt,
     net::{IpAddr, Ipv4Addr, SocketAddr},
     sync::Arc,
     time::{Duration, SystemTime},
@@ -32,8 +33,19 @@ use crate::aggregator::TaskId;
 pub struct TasksLayer {
     tx: mpsc::Sender<Event>,
     flush: Arc<aggregator::Flush>,
-    spawn_callsites: Callsites,
-    waker_callsites: Callsites,
+
+    /// Set of callsites for spans representing spawned tasks.
+    ///
+    /// For task spans, each runtime these will have like, 1-5 callsites in it, max, so
+    /// 16 is probably fine. For async operations, we may need a bigger callsites array.
+    spawn_callsites: Callsites<16>,
+
+    /// Set of callsites for events representing waker operations.
+    ///
+    /// 32 is probably a reasonable number of waker ops; it's a bit generous if
+    /// there's only one async runtime library in use, but if there are multiple,
+    /// they might all have their own sets of waker ops.
+    waker_callsites: Callsites<32>,
 }
 
 pub struct Server {
@@ -308,6 +320,19 @@ where
     }
 }
 
+impl fmt::Debug for TasksLayer {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_struct("TasksLayer")
+            // mpsc::Sender debug impl is not very useful
+            .field("tx", &format_args!("<...>"))
+            .field("tx.capacity", &self.tx.capacity())
+            .field("flush", &self.flush)
+            .field("spawn_callsites", &self.spawn_callsites)
+            .field("waker_callsites", &self.waker_callsites)
+            .finish()
+    }
+}
+
 impl Server {
     // XXX(eliza): why is `SocketAddr::new` not `const`???
     pub const DEFAULT_IP: IpAddr = IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1));