search

DataDog / dd-trace-py

Datadog Python APM Client
https://ddtrace.readthedocs.io/
Other
542 stars 411 forks source link

loguru patch: ensure only dd info is extracted to top level #9666

Closed shahar-gamliel closed 2 months ago

shahar-gamliel commented 3 months ago

Summary of problem

Which version of dd-trace-py are you using?

2.9.2

Which version of pip are you using?

irrelevant

Which libraries and their versions are you using?

loguru 0.7.2

How can we reproduce your problem?

When using the loguru patch, this is the current implementation of the code:

  1. Pass record["extra"] as the event_dict argument to _tracer_injection()
  2. Update event_dict with dd-trace attributes
  3. Update the record's top level with the returned dict.

This is causing an unintended behavior of also extracting everything that's in the extra to the top-level (and not only the dd attributes)

What is the result that you get?

As a result, we're getting unrelated extra attributes in the top level, which is messing with how we format logs.

What is the result that you expected?

I'm not sure why it's intentional that dd attributes need to be top-level, but I'd expect only them to be in top level and not other unrelated attributes that happen to be in extra.

I'd open a PR for it but it seems like it requires a permission which I don't have :(

emmettbutler commented 3 months ago

Thanks for the report, @shahar-gamliel. If I understand correctly, you're suggesting a change like the one in https://github.com/DataDog/dd-trace-py/pull/9676. Do you agree?

shahar-gamliel commented 3 months ago

Hi @emmettbutler! Yes, the change suggested in #9676 will solve the issue. Note that the record is being updated twice in that file (where you changed and a few lines below), so you might want to change that as well. I'm not sure why the dd attributes need to both be in the extra and in top level, is there a specific reason for it? Thank you very much for the quick response here 🙏🏼

shahar-gamliel commented 3 months ago

@emmettbutler created a PR to make the changes in the function itself. I meant to do it on a forked repo, but it created them on this one instead. Sorry, I'm new to open source 😅