open-telemetry / opentelemetry-swift

OpenTelemetry API for Swift
https://opentelemetry.io/docs/instrumentation/swift/
Apache License 2.0
221 stars 145 forks source link

ActivityContextManger does not propagate parents across os_activities #590

Closed bryce-b closed 1 month ago

bryce-b commented 2 months ago

Issue

When creating a span and setting it as the parent via the context manager it would be expected that a network request made during that parent span would be attributed to it as a child. This is not the case however. It seems that os_activity will create a new activity when that network request is spawned, but the context is not propagated.

Discussion

Should the ActivityContextManger attempt to propagate context when a new os_activity is spawned setting a parent in the new context? Is there a way to determine if an os_activity is scoped from another?

@mamunto to add addition context.

mamunto commented 2 months ago

Thanks @bryce-b for putting things up.

I guess we want to make sure how ActivityContextManager will work when you have concurrent situation like this:

Code block 1:

 let parentTrace = Trace.startTrace() 
 apiService.makeRequest {
     switch $0
 case .success:
     // do some work or even another chain call
     parentTrace.finish()
 case .failure:
     // handle error
     parentTrace.finish(with error)
 }

Code block 2:

 let parentTrace = Trace.startTrace() 
 apiService.makeRequest {
     switch $0
 case .success:
     // do some work or even another chain call
     parentTrace.finish()
 case .failure:
     // handle error
     parentTrace.finish(with error)
 }

Now both code blocks start in the same CPU cycle, can ActivityContextManager make sure each child of each block will have a proper parent? In our case, it was inconsistent. Are there any particular steps or instructions we have to follow to make sure regardless of where and when span creation comes it will guarantee the proper parenting? I also tried following way for concurrent scenario but didn’t help:

DispatchQueue.global().async {
    let parentSpan = Trace.startSpan(
        .uiLoad,
        context: .someContext1,
        operation: "some_operation_1"
    )
    let semaphore = DispatchSemaphore(value: 0)
    apiService.makeRequest {
        switch $0
        case .success:
            // do some work or even another chain call
        case .failure:
            // handle error
    }
    semaphore.wait()
    parentSpan.finish()
}
bryce-b commented 2 months ago

Hey @mamunto, are you calling .setActive(true) when you're building your span? This might be why it's not working.

nachoBonafonte commented 2 months ago

By default a new created span (as the one created in a network request when it is instrumented) takes the active span of its execution context as the parent

bryce-b commented 1 month ago

We resolved this during the SIG meeting. It was not an issue with the SDK. The issue was manually instrumenting network requests not using the build-in URLSessionInstrumentation.

Note: you could resolve this by using async await which could be wrapped with a span in-line. (not certain however.)