use onEnding to add baggage keys that are added while the span is active

[zeitlinger]

Fixes #1632

[trask]

I'm not sure there is any difference? (sort of related to open-telemetry/opentelemetry-java-instrumentation#12952)

[zeitlinger]

I'm not sure there is any difference? (sort of related to open-telemetry/opentelemetry-java-instrumentation#12952)

can you elaborate?

[trask]

can you elaborate?

sorry, I'm wondering why this change is needed? can you give a concrete example of when this implementation would have different behavior from existing implementation?

[zeitlinger]

sorry, I'm wondering why this change is needed? can you give a concrete example of when this implementation would have different behavior from existing implementation?

When a baggage item is added after span start:

  Span span = sdk.getTracer("test").spanBuilder("test").startSpan();

      try (Scope unused = span.makeCurrent()) {
        try (Scope scope = Baggage.current().toBuilder().put("key", "value").build()
            .makeCurrent()) {
          span.end();
        }
      }

[trask]

yeah, but that doesn't work when we follow open-telemetry/opentelemetry-java-instrumentation#12952

[zeitlinger]

yeah, but that doesn't work when we follow open-telemetry/opentelemetry-java-instrumentation#12952

OK - so you're saying that this feature should not be implemented because it would not work for users that use the agent - and that would be surprising?

[trask]

so you're saying that this feature should not be implemented because it would not work for users that use the agent

yeah, I'm not convinced we should make this change since I think "best practice" is to end span after closing scope

also I realized this change will cause the processor to break for people who are passing context manually instead of using thread local

[zeitlinger]

so you're saying that this feature should not be implemented because it would not work for users that use the agent

yeah, I'm not convinced we should make this change since I think "best practice" is to end span after closing scope

also I realized this change will cause the processor to break for people who are passing context manually instead of using thread local

I think that's a good argument - I'll add it to the issue