minor: missing descriptions of the method and newMetadata arguments

👍

should have nullable values forged

Yes I know, I'll deal with those in a future PR (when those are actually needed)

I would propose to add a TODO at the beginning of this file and DatadogCoreTest as well saying we should handle those commented lines (unlikely we will forget, but still)

Yes good point

thought: in this particular case callback is invoked synchronously, but in general API definition says it may be invoked in async manner, meaning that by the time it is invoked we may have other consent -> different orchestrator, not the one we captured for invocation. We need to be aware about that.

(...) by the time it is invoked we may have other consent -> different orchestrator, not the one we captured for invocation

Not sure if I understand this @0xnm 🤔💭. We get the orchestrator based on the immutable datadogContext passed as an parameter to writeCurrentBatch(). Then, any time later we receive a callback which provides us API (BatchWriter) to writing data. The write strategy will respect the consent value from datadogContext.

From the caller's POV, IMO everything should be consistent with this implementation: the caller orders write for given consent value and it knows that BatchWriter will respect it. No?

I mean that generally the API contract is that this callback may be invoked in async manner (if I got it right). My point is that what if user changed a consent in the timeframe between the moment we received an orchestrator and the moment we use it in the BatchWriter. Then event may be written to the location which belongs to the old consent.

This is quite hypothetical case (and may be quite an edge case), but this gist of it is that imo we need to keep in mind the possible SDK state changes with async invocations.

This is an edge case that is not something we want to dig into. The time frame for such a concurrency issue will on most case something smaller than 500ms, which is totally acceptable.

I get the point now @0xnm 👌. Agree, it's kind of edge case, but it can be clearly visible in such user situation:

// all synchronous, on a single thread:
Datadog.set(trackingConsent: .granted)
logger.info("granted log")
Datadog.set(trackingConsent: .pending)

In such case, the "granted log" should be sent by the SDK. This is however a question on how we will further synchronise things in higher implementation (even higher than Storage level). The RFC recommends processing all user calls through context thread first (see Threading in DDCore) to support exactly these edge cases.