Parse UUID received in header

matusvalo commented 2 years ago

Correlation ID UUID received from header should be converted to hex to be equal to Correlation ID UUID generated by the library.

Fixes #12

sondrelg commented 2 years ago

Thanks for opening the issue and this PR @matusvalo!

I don't think we can accept this PR as it is, because it breaks backwards compatibility for what's a pretty small change, but more importantly, because changing the inputs like this might actually make it harder for users to trace requests across their projects.

To illustrate: if frontend A sends X-Request-ID: 611AEE7D-1206-4E03-A2A6-97C238D982FD and we transform it on the way in, you would no longer be able to search for this request ID and find logs from both projects. Now you need to perform two searches, and you would need to transform the ID yourself.

I still think your issue is valid though, and I think we could solve it by rewriting the log filter like this:

+def correlation_id_filter(uuid_length: Optional[int] = None):
   class CorrelationId(Filter):
       def filter(self, record) -> bool:
           cid = correlation_id.get()

+           if uuid_length is not None and cid:
+               record.correlation_id = cid[:uuid_length]
+           else:
+               record.correlation_id = cid

           return True

   return CorrelationId

This would drop the assumption of the correlation ID being 32 characters and would behave correctly in both cases.

Do you see any drawbacks? Or do you have other reasons for wanting to transform incoming values?

matusvalo commented 2 years ago

Do you see any drawbacks? Or do you have other reasons for wanting to transform incoming values?

@sondrelg thank you for your feedback. I think your arguments make sense. I will create a new PR which will supersedes this one with your proposal.

matusvalo commented 2 years ago

Closing in favour of #14

snok / asgi-correlation-id

Parse UUID received in header #13