Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adding support for aioredis (#534) #552

Closed
wants to merge 1 commit into from
Closed

adding support for aioredis (#534) #552

wants to merge 1 commit into from

Conversation

yoav-orca
Copy link

@yoav-orca yoav-orca commented Aug 5, 2020

An initial PR for adding support aioredis, ref #534 I have encountered an unfortunate limitation.
When using:

# view code

async def redis_pool():
    # Redis client bound to pool of connections (auto-reconnecting).
    redis = await aioredis.create_redis_pool(
        'redis://localhost')
    await redis.set('my-key', 'value')
    val = await redis.get('my-key')
    print(val)

    # gracefully closing underlying connection
    redis.close()
    await redis.wait_closed()

def index(request):
    loop = get_event_loop()
    loop.run_until_complete(redis_pool())

Looking at the trace it seems that scout doesn't recognize that the task is coming from the same thread (the trace is missing the aioredis) segment.

This has already an issue #469

When using async_to_sync it works

@yoav-orca
Copy link
Author

@adamchainz what do you think?

adamchainz pushed a commit that referenced this pull request Sep 1, 2020
Rebuild of #552 with tracking in an aioredis-specific instrument submodule.
@adamchainz adamchainz mentioned this pull request Sep 1, 2020
@adamchainz
Copy link
Contributor

Hi @yoav-orca

Sorry I didn't respond until now - I went on holiday and your PR was one of many things to respond to.

Thanks for making this. It has certainly exposed the places in aioredis to instrument.

Yes there's still the problem from #469. This needs more research to overcome before we can merge anything.

One thing about your PR - our instruments are per library rather than per "technology", so this should be an "aioredis" instrument. Rather than push to your master branch, I've done this in #562 (maintaining your orignal authorship). As such I'm going to close this PR and continue working on the other PR.

We have some other higher priority work around making the launching the core agent more stable, which I'll be addressing first, but I should find time in the next few weeks to revisit aioredis.

Thanks,

Adam

@adamchainz adamchainz closed this Sep 1, 2020
adamchainz pushed a commit that referenced this pull request Sep 3, 2020
Rebuild of #552 with tracking in an aioredis-specific instrument submodule.
adamchainz pushed a commit that referenced this pull request Sep 3, 2020
Rebuild of #552 with tracking in an aioredis-specific instrument submodule.
tim-schilling pushed a commit that referenced this pull request Apr 15, 2021
Rebuild of #552 with tracking in an aioredis-specific instrument submodule.
tim-schilling pushed a commit that referenced this pull request May 4, 2021
Rebuild of #552 with tracking in an aioredis-specific instrument submodule.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants