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

Develop/conditional server span pyramid #869

Conversation

sanketmehta28
Copy link
Member

@sanketmehta28 sanketmehta28 commented Jan 19, 2022

Description

This PR will fix the issue of creating a separate span when pyramid application is instrumented along with any other framework like asgi or wsgi. This fix will extract the span from context first it exists and then create a child span which is expected behavior

Fixes #449

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

I have added a separate unit test in test_automatic.py for pyramid instrumentation

  • Test A:
  • SERVER span is provided in the context.
  • then test client send the request to instrumented pyramid application which will create its own span
  • check whether the newly created span is child of the existing span and of type INTERNEL

Does This PR Require a Core Repo Change?

  • 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

@sanketmehta28 sanketmehta28 requested a review from a team January 19, 2022 12:04
@linux-foundation-easycla
Copy link

CLA Not Signed

sanketmehta28 added a commit to sanketmehta28/opentelemetry-python-contrib that referenced this pull request Jan 19, 2022
@linux-foundation-easycla
Copy link

CLA Not Signed

Copy link
Contributor

@owais owais left a comment

Choose a reason for hiding this comment

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

LGTM. Please take care of the nit-picky comment :)

@linux-foundation-easycla
Copy link

CLA Not Signed

@linux-foundation-easycla
Copy link

CLA Not Signed

@sanketmehta28 sanketmehta28 force-pushed the develop/conditional_server_span_pyramid branch from c24a480 to c1ce2d6 Compare January 21, 2022 12:47
@linux-foundation-easycla
Copy link

CLA Not Signed

ashu658 and others added 3 commits January 23, 2022 01:17
* Making span as internal for falcon in presence of a span in current context

* Updating changelog

* Fixing lint and generate build failures

* Resolving comments: Converting snippet to re-usable function

* Fixing build failures

* Resolving comments: Creating wrapper for start span to make internal/server span

* Rerun docker tests

* Resolving comments: Refactoring
…etry#870)

* Update CHANGELOG.md

* Fix Django 1.9 issue preventing use of MIDDLEWARE_CLASSES

Co-authored-by: Srikanth Chekuri <srikanth.chekuri92@gmail.com>
Copy link
Member

@srikanthccv srikanthccv left a comment

Choose a reason for hiding this comment

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

LGTM, non-blocking request: If you can keep the prev history as is that would be great.

@owais owais enabled auto-merge (squash) January 25, 2022 15:15
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.

Pyramid: Conditionally create SERVER spans
5 participants