[agocs]

Description

Updates the trace propagation format for botocore direct lambda invocations. The old way was:

clientContext: {
  custom:{
    _datadog: {
      x-datadog-trace-id: "",
      x-datadog-parent-id: "",
      ...
    }
  }
}

New way is:

clientContext: {
  custom: {
      x-datadog-trace-id: "",
      x-datadog-parent-id: "",
      ...
  }
}

https://docs.google.com/document/d/1pruSeesAr2bAVMtz_rUlsiBZKFoFWdB4nSq5Vuym2l8/edit?usp=sharing

Reasoning

The Go and Java runtimes only support ClientContext.custom fields of type map[string]string. C.f. :

• Custom client context parser error aws/aws-lambda-java-libs#212
• AWS lambda plugin custom context breaking with aws-lambda-java dd-trace-js#1312

In order to pass trace contexts to directly invoked Go and Java lambda functions, we can write the trace context key:values to ClientContext.custom rather than creating a ClientContext.custom._datadog object like we've been doing.

Backwards compatibility

This is, strictly speaking, a breaking change in how we add trace context to direct invocations. The Python and JS repos have already been updated to accept the new format:

• Check for the new direct invocation trace format and use that datadog-lambda-js#194
• Check for the new direct invocation trace format and use that datadog-lambda-python#146

They both check for the old and new formats.

Checklist

• Added to the correct milestone.
• Tests provided or description of manual testing performed is included in the code or PR.
• Library documentation is updated.
• Corp site documentation is updated (link to the PR).

[Kyle-Verhoog]

I'm thinking we should leave the old method behind a configuration option since a bunch of users will not be on the latest version of the serverless client(s) 🤔

Thoughts @agocs?

[agocs]

@Kyle-Verhoog I can get behind that. I considered it, but initially decided against it as this is such a niche change. Definitely happy to add that parameter though.

Which way do you think the default should go? I was thinking it should favor the new, working trace propagation format.

Oh, and is this the extent of the configuration, or is there a config file hiding somewhere I don't know about? https://docs.datadoghq.com/tracing/setup_overview/setup/python/?tab=otherenvironments#configuration

[Kyle-Verhoog]

Which way do you think the default should go? I was thinking it should favor the new, working trace propagation format.

Yup - agreed 👍

The configuration option can be added here:

dd-trace-py/ddtrace/contrib/botocore/patch.py

Lines 35 to 41 in 7e1d82a

	# Botocore default settings
	config._add(
	"botocore",
	{
	"distributed_tracing": get_env("botocore", "distributed_tracing", default=True),
	},
	)

. We should offer an environment variable too

and then documented here

dd-trace-py/ddtrace/contrib/botocore/__init__.py

Lines 16 to 20 in 7e1d82a

	Configuration
	~~~~~~~~~~~~~
	.. py:data:: ddtrace.config.botocore['distributed_tracing']

(this will show up here: https://ddtrace.readthedocs.io/en/stable/integrations.html#botocore)

Also an upgrade release note entry would be required here which can be created with reno:

$ reno new botocore

[agocs]

Awesome! I appreciate it!

[agocs]

@Kyle-Verhoog Changes made! Thanks. Should I drop a link to this in the apm-python channel?

[Kyle-Verhoog]

👍 solid tests - thanks for also keeping a test with the old setting! Just some suggestions around the user-facing aspects 🙂

[Kyle-Verhoog]

Suggested change

	"botocore", "clientcontext_custom_add_datadog_object", default=False
	"botocore", "legacy_context", default=False

Not 100% about this suggesting.. aiming toward something less verbose - I don't think the name has to divulge the implementation details here... unless we're planning in the future to change this implementation again 😬

@DataDog/apm-python any ideas for naming here?

[agocs]

invoke_with_legacy_context ?

[Kyle-Verhoog]

This description is awesome but probably a bit too verbose for users who might get lost in the details. The important part is the version support listed below. WDYT about trimming it down?

[agocs]

👍

[Kyle-Verhoog]

nice! just some last concerns about the release note but otherwise lgtm!

[Kyle-Verhoog]

this should be an upgrade-level note mentioning the change and the backwards compatibility option as users using older python/node lambda client versions will have a breaking change

[Kyle-Verhoog]

👍 I think I'm good with this name.

@DataDog/apm-python thoughts?

[brettlangdon]

👍🏻

[Kyle-Verhoog]

I'd probably trim this down as well. From the user standpoint they just need to know that context management support has been added for newer lambda libraries and that there is a legacy configuration option in-case they wish to run an older version.

👍 for mentioning the specific versions of each library.

Would also be good to explicitly show the configuration options, something like:

Legacy support for older libraries is available with ddtrace.config.botocore.invoke_with_legacy_context = True or by setting the environment variable DD_BOTOCORE_INVOKE_WITH_LEGACY_CONTEXT=true

[Kyle-Verhoog]

I'd like to see it explicitly mentioned that the propagation will break for users using datadog-lambda-python < v41 or datadog-lambda-js < v3.57.0 first and then mention the introduction of the legacy setting that can be used to support these older versions.

[Kyle-Verhoog]

Just one more thought on the release note!

[Kyle-Verhoog]

I'd like to see it explicitly mentioned that the propagation will break for users using datadog-lambda-python < v41 or datadog-lambda-js < v3.57.0 first and then mention the introduction of the legacy setting that can be used to support these older versions.

[Kyle-Verhoog]

👏 thanks for bearing with my nitty documentation feedback @agocs!