service.name is set to None because of elasticapm_service_name
Atheuz opened this issue · 3 comments
Hi,
If you have an application that adds a key like this to its log:
{"service": {"version": 1.0.0, "name": "myapp", "environment": "dev"}}
Then you end up having service.name be set to None and ultimately be deleted from the output, because of this line:
ecs-logging-python/ecs_logging/_stdlib.py
Line 227 in 9f35b52
Maybe this should instead set service.name to prefer elasticapm_service_name, but fall back to service.name if it already exists and isn't None, i.e.:
extras["service.name"] = extras.pop("elasticapm_service_name", extras.get("service.name", None))
I'm not using Elastic APM and ran into this issue.
Great find @Atheuz! Another option would be
extras.setdefault("service.name", extras.pop("elasticapm_service_name", None))
This would only set "service.name" if it isn't set already. Maybe it would even be a good idea to check for the presence of any of the five fields that we get from Elastic APM, and only set them if none of them are set already. This would avoid a confusing scenario where only some of the ECS fields are from Elastic APM, but not all. What do you think @sethmlarson @basepi?
@beniwohli Definitely agree that .setdefault() is way better than an unconditional __setitem__, I'm ++ on your suggestion.