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

Change default redis SpanKind to CLIENT #965

Merged
merged 2 commits into from
Aug 5, 2020

Conversation

akoumjian
Copy link
Contributor

Description

This is a tiny change to set the SpanKind of the redis instrumentor to CLIENT. This is a small semantic change I'm suggesting so that tools which display spans based on SpanKind can show these connections in the correct context.

Type of change

Minor enhancement.

How Has This Been Tested?

One test was added to validate the SpanKind and also added validation for span.name.

  • Test the property of the Span.SpanKind

Checklist:

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

@akoumjian akoumjian requested a review from a team August 2, 2020 18:27
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 2, 2020

CLA Check
The committers are authorized under a signed CLA.

@akoumjian akoumjian changed the title WIP: Change default redis SpanKind to CLIENT Change default redis SpanKind to CLIENT Aug 2, 2020
@akoumjian
Copy link
Contributor Author

Not sure how to fix that failing test, does not seem related.

Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! If you could resolve the conflicts, i'm happy to approve and merge.

@akoumjian
Copy link
Contributor Author

@codeboten Conflicts resolved.

Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for resolving the conflicts!

@codeboten codeboten merged commit 9558900 into open-telemetry:master Aug 5, 2020
@akoumjian akoumjian deleted the redis_spankind branch August 5, 2020 15:06
srikanthccv pushed a commit to srikanthccv/opentelemetry-python that referenced this pull request Nov 1, 2020
* feat(opentelemetry-exporter-jaeger): http sender

* fix: linter

* fix(opentelemetry-exporter-jaeger): adds header to avoid infinity loop

* test(opentelemetry-exporter-jaeger): verify http sender usage

* refactor(opentelemetry-exporter-jaeger): checks if endpoint is setten

* feat(opentelemetry-exporter-jaeger): adds nock as dev dependency

* test(opentelemetry-exporter-jaeger): adds tests to check req header

* fix: tests variable usage

* refactor(opentelemetry-exporter-jaeger): removes config parameter change

* fix: linter

* fix: env var was never called

Co-authored-by: Mayur Kale <mayurkale@google.com>
Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com>
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.

3 participants