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

Allow to modify the computed Span name via callback. #1431

Closed
wants to merge 2 commits into from

Conversation

pilhuhn
Copy link

@pilhuhn pilhuhn commented Nov 6, 2022

Description

With Redis and Databases in a traces, there is no easy way from the span name to see if a "SELECT" is going to Redis or the database.

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

  • TestRedis::test_name_callback

Does This PR Require a Core Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • [?] Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

@pilhuhn pilhuhn requested a review from a team November 6, 2022 12:14
@srikanthccv
Copy link
Member

We removed the name_callback and added request_hook and response_hook. They can be used to change the default span name if needed. You could do something like this.

def request_hook(span, instance, args, kwargs):
  if span.is_recording():
    span.update_name("my-custom-name")

@pilhuhn
Copy link
Author

pilhuhn commented Nov 6, 2022

Hm. I got the recommendation to add the name_callback on Slack (https://cloud-native.slack.com/archives/C01PD4HUVBL/p1666969756061799). Will try your suggestion.

@srikanthccv
Copy link
Member

Take a look at #411. We are getting rid of it in favour of request/response hooks. It will be removed eventually for requests instrumentation. Aaron might not be aware of this detail. And here is another rejected proposal #1175. Please try the suggested method and let us know if that doesn't solve your issue.

@pilhuhn
Copy link
Author

pilhuhn commented Nov 7, 2022

Ok, the following works for me:

    def redis_response_hook(span, instance, response):
      if span.is_recording():
          name = span.name
          name = "Redis " + name
          span.update_name(name)

    from opentelemetry.instrumentation.redis import RedisInstrumentor
    RedisInstrumentor().instrument(response_hook=redis_response_hook)

@pilhuhn pilhuhn closed this Nov 7, 2022
@pilhuhn pilhuhn deleted the redis-span-name-callback branch November 7, 2022 10:42
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