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.

Sign up for GitHub

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

Jump to bottom

Tracing update #7252

Merged
merged 23 commits into from
Sep 23, 2019
Merged

Tracing update #7252

merged 23 commits into from
Sep 23, 2019

Conversation

lmazuel
Copy link
Member

@lmazuel lmazuel commented Sep 12, 2019

Refactoring

  • Use get_parent more intensively to avoid code redondancy
  • Renamed link to link_from_headers, and make link more generic by taking a traceparent
  • Do not auto-detect nested decorated method, but pass a 'merge_span' instead

Features

  • Add contextmanager to AbstractSpan and Opencensus impl
  • Add Kind to AbstractSpan and Opencensus impl
  • Set Opencensus "Span Status" on exception (decorator + policy) if context manager used
  • Add a get_trace_parent to AbstractSpan and Opencensus impl

This PR also apply merge_span when necessary in storage client

@lmazuel lmazuel requested a review from johanste September 12, 2019 23:19
@Azure Azure deleted a comment from AutorestCI Sep 13, 2019
@Azure Azure deleted a comment from AutorestCI Sep 16, 2019
@Azure Azure deleted a comment from AutorestCI Sep 18, 2019
@Azure Azure deleted a comment from AutorestCI Sep 18, 2019
@Azure Azure deleted a comment from AutorestCI Sep 18, 2019
@bryevdv
Copy link
Contributor

bryevdv commented Sep 19, 2019

Much simpler and cleaner now +1

@Azure Azure deleted a comment from AutorestCI Sep 20, 2019
johanste
johanste approved these changes Sep 20, 2019
View reviewed changes
Copy link
Member

@johanste johanste left a comment

Choose a reason for hiding this comment

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

Other than the exception/failure question, looks good!

@Azure Azure deleted a comment from AutorestCI Sep 20, 2019
@Azure Azure deleted a comment from AutorestCI Sep 23, 2019
@AutorestCI
Copy link
Contributor

AutorestCI commented Sep 23, 2019 •
edited
Loading

(message created by the CI based on PR content)

Installation instruction

Package azure-core

You can install the package azure-core of this PR using the following command:
pip install "git+https://github.com/Azure/azure-sdk-for-python@tracing_update#egg=azure-core&subdirectory=azure-core"

You can build a wheel to distribute for test using the following command:
pip wheel --no-deps "git+https://github.com/Azure/azure-sdk-for-python@tracing_update#egg=azure-core&subdirectory=azure-core"

If you have a local clone of this repository, you can also do:

  • git checkout tracing_update
  • pip install -e ./azure-core

Or build a wheel file to distribute for testing:

  • git checkout tracing_update
  • pip wheel --no-deps ./azure-core

Package azure-storage-blob

You can install the package azure-storage-blob of this PR using the following command:
pip install "git+https://github.com/Azure/azure-sdk-for-python@tracing_update#egg=azure-storage-blob&subdirectory=azure-storage-blob"

You can build a wheel to distribute for test using the following command:
pip wheel --no-deps "git+https://github.com/Azure/azure-sdk-for-python@tracing_update#egg=azure-storage-blob&subdirectory=azure-storage-blob"

If you have a local clone of this repository, you can also do:

  • git checkout tracing_update
  • pip install -e ./azure-storage-blob

Or build a wheel file to distribute for testing:

  • git checkout tracing_update
  • pip wheel --no-deps ./azure-storage-blob

Package azure-storage-file

You can install the package azure-storage-file of this PR using the following command:
pip install "git+https://github.com/Azure/azure-sdk-for-python@tracing_update#egg=azure-storage-file&subdirectory=azure-storage-file"

You can build a wheel to distribute for test using the following command:
pip wheel --no-deps "git+https://github.com/Azure/azure-sdk-for-python@tracing_update#egg=azure-storage-file&subdirectory=azure-storage-file"

If you have a local clone of this repository, you can also do:

  • git checkout tracing_update
  • pip install -e ./azure-storage-file

Or build a wheel file to distribute for testing:

  • git checkout tracing_update
  • pip wheel --no-deps ./azure-storage-file

Package azure-storage-queue

You can install the package azure-storage-queue of this PR using the following command:
pip install "git+https://github.com/Azure/azure-sdk-for-python@tracing_update#egg=azure-storage-queue&subdirectory=azure-storage-queue"

You can build a wheel to distribute for test using the following command:
pip wheel --no-deps "git+https://github.com/Azure/azure-sdk-for-python@tracing_update#egg=azure-storage-queue&subdirectory=azure-storage-queue"

If you have a local clone of this repository, you can also do:

  • git checkout tracing_update
  • pip install -e ./azure-storage-queue

Or build a wheel file to distribute for testing:

  • git checkout tracing_update
  • pip wheel --no-deps ./azure-storage-queue

Direct download

Your files can be directly downloaded here:

@lmazuel
Copy link
Member Author

lmazuel commented Sep 23, 2019

CI failure is caused by #7407 and not related to this PR content. Core CI itself is green. Merging.

@lmazuel lmazuel merged commit d886a7d into master Sep 23, 2019
@lmazuel lmazuel deleted the tracing_update branch October 2, 2019 15:10
YijunXieMS pushed a commit to YijunXieMS/azure-sdk-for-python that referenced this pull request Oct 9, 2019
@lmazuel
Tracing update (Azure#7252)
1923f91 
* Refactoring tracing for clarity

* Merge span explicitly on SDK side

* Continue fixing merge span

* Add Kind to generic Span + impl

* Add contextmanager and status

* Propagate more merge_span in storage

* Docstring fixes

* Remove pointless import

* Simplify tracing implementation

* Simplify decorator implementation

* Remove SDK context

* Thread context

* Add missing dev dep

* Swap dev dep

* Happy pylint

* mypy

* mypy 2

* pylint

* Make tracing a nver fail policy

* Some ChangeLog

* Traceparent doc clarification

* pylint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@johanste johanste johanste approved these changes

@bryevdv bryevdv bryevdv approved these changes

@zezha-msft zezha-msft zezha-msft approved these changes

@annatisch annatisch Awaiting requested review from annatisch

@rakshith91 rakshith91 Awaiting requested review from rakshith91

@xiangyan99 xiangyan99 Awaiting requested review from xiangyan99

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@lmazuel @bryevdv @AutorestCI @johanste @zezha-msft