snok / asgi-correlation-id

Request ID propagation for ASGI apps
MIT License
369 stars 29 forks source link

Ability to change the format of UUID #38

Closed commentator8 closed 2 years ago

commentator8 commented 2 years ago

Hi,

I would love to be able to use ULID in place of uuid4, and sadly the choice is hardcoded, not something that can easily be worked around. Either providing something like ulid as an alternative, or allowing for the provision of a generator (/validator) method that could be injected would do the job. I'm a little hard pressed to create a PR myself, but I was wondering if this could make it onto a roadmap?

Thanks and love the package!

sondrelg commented 2 years ago

I started adding that in #15 a while back, then thought I'd wait until someone requested it before adding it.

What do you think about both a validator and a generator, like this?

app.add_middleware(
    CorrelationIdMiddleware,
    header_name='X-Request-ID',
    generator=lambda: uuid4().hex,
    validator=is_valid_uuid,
    transformer=lambda a: a,
)

If you have time, I'd love some feedback on that PR

commentator8 commented 2 years ago

I didn't follow the point you wrote about not liking initializing validators to None - I saw a default value using the uuid function. Some minor points:

Looks great to me!

sondrelg commented 2 years ago

Yeah good notes. I think the comment about validators might be outdated, from an earlier draft. Think I'll polish the PR a little bit and release a v3 release today or tomorrow 👍

commentator8 commented 2 years ago

Thanks for the really fast turnaround! I can pull directly from the branch, but before that I'll check - are you planning on releasing a version 3 anytime soon?

sondrelg commented 2 years ago

I released 3.0.0a1 an hour ago. You should be able to install it with pip install asgi-correlation-id==3.0.0a1 or with poetry add asgi-correlation-id==3.0.0a1 --allow-prereleases. Was hoping to run the pre-release for a day then release v3 tomorrow morning 👍