[Refactor] Pass uniquely owned custom providers to the GlobalMetadataProvider

[patnebe]

Context

• The additional thread-safety mechanisms/synchronisation layer around the registered custom providers seems redundant and could lead to additional overhead.
• Since the collector uses a ThreadSafeGlobalMetadataProvider', the additional synchronization around each internal member of that class might not be needed.

Changes

• Remove the Arc<Mutex<>> wrapper around the registered custom providers
• Pass uniquely owned custom providers to the GlobalMetadataProvider

Test Plan

• Existing CI checks.

[patnebe]

cc @javierhonduco As per the notes in the description above. Do you have any concerns about relaxing the thread safety requirements for the custom providers?

[javierhonduco]

Thanks for the PR! Taking a look soon, looks good overall!

[javierhonduco]

Looks good! Thanks for the change. As you mention the top-level struct mutex should be enough here. Reading the code now, a small improvement we could make is to use a read-write lock for metadata. What do you think?

[patnebe]

Looks good! Thanks for the change. As you mention the top-level struct mutex should be enough here. Reading the code now, a small improvement we could make is to use a read-write lock for metadata. What do you think?

Discussed this offline, but adding a note here for reference.

The interface for the GlobalMetadataProvider does not currently support/provide read-only vs write-only functions. All public methods on the class currently mutate the class's internal state. Hence wrapping the provider with a RwLock would probably not provide any additional benefit over a good old Mutex (which is the current implementation).

We ultimately decided that the current approach can be left as is, and revisited if there's a need to improve it further.