Repository: diesel-rs/diesel
File: diesel/src/connection/instrumentation.rs (Rust)

Code before:
    }
}

impl<T> Instrumentation for Option<T>
where
    T: Instrumentation,
{
    fn on_connection_event(&mut self, event: InstrumentationEvent<'_>) {
        if let Some(i) = self {
            i.on_connection_event(event)
        }
    }
}

#[diesel_derives::__diesel_public_if(
    feature = "i-implement-a-third-party-backend-and-opt-into-breaking-changes"
)]
/// An optional dyn instrumentation.
/// For ease of use, this type implements [`Deref`] and [`DerefMut`] to `&dyn Instrumentation`,
/// falling back to a no-op implementation if no instrumentation is set.
pub(crate) struct DynInstrumentation {
    /// zst
    no_instrumentation: NoInstrumentation,
    inner: Option<Box<dyn Instrumentation>>,
}
impl Deref for DynInstrumentation {
    type Target = dyn Instrumentation;

    fn deref(&self) -> &Self::Target {
        self.inner.as_deref().unwrap_or(&self.no_instrumentation)
    }
}
impl DerefMut for DynInstrumentation {
    fn deref_mut(&mut self) -> &mut Self::Target {
        self.inner
            .as_deref_mut()
            .unwrap_or(&mut self.no_instrumentation)
    }
}
impl DynInstrumentation {
    #[diesel_derives::__diesel_public_if(
        feature = "i-implement-a-third-party-backend-and-opt-into-breaking-changes"
    )]
    pub(crate) fn default_instrumentation() -> Self {
        Self {
            inner: get_default_instrumentation(),
            no_instrumentation: NoInstrumentation,
        }
    }

    #[diesel_derives::__diesel_public_if(

Review comment (refactor):
Can we add the following part of your PR description as comment here?

> The DynInstrumentation type is useful because without it we actually did tend to return &mut Option<Box<dyn Instrumentation>> as &mut dyn Instrumentation from connection.instrumentation(), so downcasting would have to be done in these two steps by the user, which is counter-intuitive and doesn't seem ideal inside diesel itself either (because it induces extra dyn dispatch levels).

That provides the necessary context on why we use this abstraction instead of just `Option<Box<dyn Instrumentation>>` as before.

Also how about naming this type `OptionalInstrumentation` instead?

Code after:
    }
}

impl<T> Instrumentation for Option<T>
where
    T: Instrumentation,
{
    fn on_connection_event(&mut self, event: InstrumentationEvent<'_>) {
        if let Some(i) = self {
            i.on_connection_event(event)
        }
    }
}

#[diesel_derives::__diesel_public_if(
    feature = "i-implement-a-third-party-backend-and-opt-into-breaking-changes"
)]
/// An optional dyn instrumentation.
///
/// For ease of use, this type implements [`Deref`] and [`DerefMut`] to `&dyn Instrumentation`,
/// falling back to a no-op implementation if no instrumentation is set.
///
/// The DynInstrumentation type is useful because without it we actually did tend to return
/// (accidentally) &mut Option<Box> as &mut dyn Instrumentation from connection.instrumentation(),
/// so downcasting would have to be done in these two steps by the user, which is counter-intuitive.
pub(crate) struct DynInstrumentation {
    /// zst
    no_instrumentation: NoInstrumentation,
    inner: Option<Box<dyn Instrumentation>>,
}

impl Deref for DynInstrumentation {
    type Target = dyn Instrumentation;

    fn deref(&self) -> &Self::Target {
        self.inner.as_deref().unwrap_or(&self.no_instrumentation)
    }
}

impl DerefMut for DynInstrumentation {
    fn deref_mut(&mut self) -> &mut Self::Target {
        self.inner
            .as_deref_mut()
            .unwrap_or(&mut self.no_instrumentation)
    }
}

impl DynInstrumentation {
    #[diesel_derives::__diesel_public_if(
        feature = "i-implement-a-third-party-backend-and-opt-into-breaking-changes"
    )]