Add analyzed span configuration option for Celery

[ericmustin]

Summary

This PR addresses Open Issue 1217, by adding app analytics support for celery worker and producer spans. Previously it was not possible to enable spans created by Celery instrumentation as analyzed spans for use in app analytics. The only workaround was relying on the datadog-agent configuration setting which is brittle and decoupled from application settings, making it difficult to maintain. This PR exposes an environment variable DD_CELERY_ANALYTICS_ENABLED at the application level to enable analyzed spans for celery.

Example Usage

DD_CELERY_ANALYTICS_ENABLED=true ddtrace-run python app.py

Notes

• long time listener first time caller. Tried to follow the documentation and test style of similar integrations where app analytics is disabled by default but can be enabled optionally via env var. Please let me know if there's anything I missed or any way y'all would prefer this documented. Happy to make any changes

• Additionally, it's worth noting that this option enables both worker and producer spans for app analytics. Seemed like a reasonable enough behaviour, but if you feel like there ought to be optionality to choose only worker spans or only producer spans to be enabled for app analytics, I'm happy to refactor this. I noticed that celery's configuration offers the ability to name the worker_service_name and producer_service_name separately.

• Lastly, it appears that algoliasearch, dogpile_cache, jinja2, and mako also cannot enable app analytics at the moment. If this approach looks good I would be happy to try to tackle those as well either in this pr or in a separate one. Just figured I would grab celery first since there's an open issue on it.

[Kyle-Verhoog]

Just one comment and I think it's good to go!

👍 for docs updates
👍 for great set of tests

[Kyle-Verhoog]

I think we only want to set the tag here if config.celery.get_analytics_sample_rate() is not None. 🙂

[ericmustin]

tbh I was just borrowing how the other integrations do it, like boto for instance, which just naively sets the return of get_analytics_sample_rate(), which would be None in the case that analytics_enabled is not set or set to false.

Happy to change it if setting a tag to None is an antipattern

dd-trace-py/ddtrace/settings/integration.py

Lines 82 to 98 in 4653e97

	def get_analytics_sample_rate(self, use_global_config=False):
	"""
	Returns analytics sample rate but only when integration-specific
	analytics configuration is enabled with optional override with global
	configuration
	"""
	if self._is_analytics_enabled(use_global_config):
	analytics_sample_rate = getattr(self, 'analytics_sample_rate', None)
	# return True if attribute is None or attribute not found
	if analytics_sample_rate is None:
	return True
	# otherwise return rate
	return analytics_sample_rate
	# Use `None` as a way to say that it was not defined,
	# `False` would mean `0` which is a different thing
	return None

[Kyle-Verhoog]

Hmm fair enough. However I think it is an antipattern since TMU None will get stringified to 'None' which is meaningless for this tag 😛.

I'm sure it gets handled and dropped somewhere in the backend but I think it's cleaner if we just don't set it. Less data to process and pass around 🙂

[ericmustin]

makes sense, appreciate the clarification, updated.

[Kyle-Verhoog]

Thanks!! 😄