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

[ServiceBus] Update tracing #29995

Merged

Conversation

pvaneck
Copy link
Member

@pvaneck pvaneck commented Apr 20, 2023

Various tracing updates in this PR:

  • A span will now be created upon calls to the service that settle messages.
  • Additional attributes added to spans:
    • messaging.system - messaging system (i.e., servicebus)
    • messaging.operation - type of operation (i.e., publish, receive, or settle)
    • messaging.batch.message_count - number of messages sent or received (if more than one)
  • All send spans now contain links to message spans. Now, message spans will no longer contain a link to the send span.

Maintained legacy attribute names for now.

Reference overview about tracing for AMQP

Part of: #25838

Ran some perf tests with tracing enabled (Opentelemetry SDK Tracer Provider) and disabled. Numbers below are from an average of five runs.

SendQueueMessageBatchTest --message-size 1024 --batch-size 100
Tracing Off: 1102 ops/s
Tracing On: 1031 ops/s
Percent Change: -6.4%

ReceiveQueueMessageBatchTest --message-size 2000 --num-messages 50 --preload 10000
Tracing Off: 905.7 ops/s
Tracing On: 890.9 ops/s
Percent Change: -1.6%

@azure-sdk
Copy link
Collaborator

API change check

APIView has identified API level changes in this PR and created following API reviews.

azure-servicebus

- "Send" span now contains links to message spans.
- Receive span is now kind CLIENT instead of CONSUMER.
- Added span creation logic for settlement methods.
- Attribute names were updated to align with distributed
  tracing conventions.
- Some span named renamed to align with other SDKs.
- Receive spans now have more accurate start times.

Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>
Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>
Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>
Copy link
Member

@swathipil swathipil left a comment

Choose a reason for hiding this comment

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

lgtm, thanks @pvaneck! (left a couple comments + tested locally with sync/async otel samples, which now work with these changes)

Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>
This is slated to be removed in favor of `messaging.destination.name`
for everything. Here, we maintain use of the legacy attribute name
`message_bus.destination`.

Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>
@pvaneck
Copy link
Member Author

pvaneck commented May 4, 2023

/azp run python - servicebus - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@swathipil
Copy link
Member

/azp run python - servicebus - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@swathipil swathipil merged commit 170fe74 into Azure:feature/servicebus/pyproto May 5, 2023
29 checks passed
swathipil added a commit to swathipil/azure-sdk-for-python that referenced this pull request May 5, 2023
* [ServiceBus] Update tracing

- "Send" span now contains links to message spans.
- Receive span is now kind CLIENT instead of CONSUMER.
- Added span creation logic for settlement methods.
- Attribute names were updated to align with distributed
  tracing conventions.
- Some span named renamed to align with other SDKs.
- Receive spans now have more accurate start times.

Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>

* Refactor tracing utils

Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>

* Remove unneeded arg from trace_message

Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>

* update changelog

Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>

* Remove use of `messaging.source.name`

This is slated to be removed in favor of `messaging.destination.name`
for everything. Here, we maintain use of the legacy attribute name
`message_bus.destination`.

Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>

* remove test-resources.bicep from stress

---------

Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>
Co-authored-by: swathipil <swathip@microsoft.com>
swathipil added a commit that referenced this pull request May 7, 2023
* upstream + sb pyamqp

* pyamqp from eventhub

* asyncio markers on async tests

* remove recordings

* updating sender and session from main

* missing req

* [Service Bus] Performance Tests (#28399)

* delete t1 tests

* perf tests

* fix size

* few small fixes for topic

* perf tests redone

* stream tests

* update logic to handle recv & del

* restructure classes

* formatting

* preload only delta messages

* fixes

* rename test files

* swathis comments

* perf bicep

* fix transport & remove shared

* if peeklock, then complete

* make batch receive batchperftests

* restructure classes

* fix

* fixes for max_count

* fix wording

* minor clean up

* move add args to mixin

* [ServiceBus] Iterator Support (#28558)

* iterator

* add todo

* remove logger

* tests

* skip serialization and pyamp transport errors

* add in keep-alive for releasing messages

* try this for client.py

* trying to fix iterator vs normal receiving timeout for releasing messages

* remove part of test failing for sync release -- doesnt work on pyamqp

* make message yeilding a while loop

* copying changes to async

* fixing async release test

* message_received was set incorrectly/ diff than uamqp

* fix async

* fixing closing order logic

* receive_contxt, fix test

* another receive_context

* asyncio.lock

* receiver_context

* try to ignore sync for now

* async with lock

* receive_context

* remove print statements

* unskip sync

* fix pylint and mypy

* skip tests except for failing one

* mark not mock

* socket read timeout set to 1 but was .2 on sync

* run all tests with new timeout

* pr comments

* pr comments

* typing add back

* remove todo

* time to live - ttl

* async init

* async init

* pr comments

* Revert "pr comments"

This reverts commit f8c96f2.

* pr comments - lock rename

* pr comments - remove timeout setting for uamqp

* tests

* set link credit in connection listen on keep alive

* link async

* change pop to get

* try flow before connection

* test

* link credit not bein kept bc of flow in client_run

* if link credit is 0, reset it

* add wait_time to tests

* time override test

* Connection to Link Error

* sleep fix

* link_credit

* missing _ in test

* remove boolean flag

* need actibvity timestamp in yeild message

* missing "_"

* stamp ->timestamp

* dont fix EH here

* formatting pylint

* sock timeout async

* whitespace

* remove __aiter__

* need iter

* remove todo

* todo

* whitespace

* iter_context

* remove self

* tests - remove #pytest.skip

* add timeouts to constants

* merge together if statements

* timeout

* missing if

* pylint

* pylint

* whitespace

* todo

* if statement :)

* pylint/remove whitespace

* [ServiceBus] merge EH pyamqp into SB pyamqp (#29223)

* [TEMPORARY] adding eh _pyamqp folder

* [TEMP] add _pyamqp/aio

* undo removing client lock

* lint

* [ServiceBus] update pyproto b1 version/changelog/readme (#29251)

* [ServiceBus] update changelog 7.9.0b1 (#29267)

* Increment version for servicebus releases (#29268)

Increment package version after release of azure-servicebus

* [ServiceBus] Pyamqp Changes from EH in SB (#29499)

* frame fix sync

* frame fix async

* [ServiceBus] uamqp/pyamqp switch (#28512)

- Added _pyamqp_transport.py/_uamqp_transport.py, which contain all corresponding uamqp/pyamqp code.
TODO:
- tests:
  - [ ] manually create ServiceBusMessageBatch and set client to `uamqp_transport=True` when sending.

* [ServiceBus] Files for SB Perf Tests (#29503)

* files for perf test CI

* fix args, vals based on comments

* remove unused params

* add batch size back in to perf test

* add in add_arguments in to send base

* adjust message size

* fix

* [Service Bus] Fix System.Byte[] Not Supported (#29670)

* add in string decode for 161

* app keys are now strings

* app keys are now strings

* remove decode

* revert changes

* decode if app props val is bytes

* remove change from pyamqp layer

* move fix in to outgoing message

* remove extra )

* fix

* mypy and pylint

* fix in back_compat

* rephrase

* [ServiceBus] Fix sb perf test (#29765)

* fixes for perf bicep

* fix env vars and params

* [ServiceBus] prep release 7.10.0b1 (#29815)

* prep for release

* update readmes

* update docs for switch

* remove uamqp dev req temporarily

* fix mypy/pylint

* add back uamqp to dev reqs

* merge main in topyamqp

* update release date

* Increment package version after release of azure-servicebus (#29881)

* [SB] Remove references to internal streaming method (#29750)

* remove streaming

* update stress

* move uamqp transport imports into client constructor (#29921)

* [ServiceBus][Perf] Fix perf tests (#30004)

Some async tests were trying to use an synchronous receiver/sender.
This fixes that so that the tests can run.

Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>

* [SB Pyamqp] stress updates (#29783)

* stress updates

* changes

* add memray to stress

* undo docker file changes

* add memray chaos

* timeoutError raise

* devred

* try log to file

* test indiv

* test indv

* updates

* tests

* logging_enable

* stress

* update

* delete

* remove changes to code

* change level

* update chart.yaml

* update to local running of indv components

* updates

* remove

* update

* update test base

* remove eh changes

* logging

* update jpb

* update docker

* update scenarios

* logging

---------

Co-authored-by: swathipil <swathip@microsoft.com>

* [ServiceBus] Update tracing (#29995)

* [ServiceBus] Update tracing

- "Send" span now contains links to message spans.
- Receive span is now kind CLIENT instead of CONSUMER.
- Added span creation logic for settlement methods.
- Attribute names were updated to align with distributed
  tracing conventions.
- Some span named renamed to align with other SDKs.
- Receive spans now have more accurate start times.

Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>

* Refactor tracing utils

Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>

* Remove unneeded arg from trace_message

Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>

* update changelog

Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>

* Remove use of `messaging.source.name`

This is slated to be removed in favor of `messaging.destination.name`
for everything. Here, we maintain use of the legacy attribute name
`message_bus.destination`.

Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>

* remove test-resources.bicep from stress

---------

Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>
Co-authored-by: swathipil <swathip@microsoft.com>

* [ServiceBus] Fix Memory Leak on Network Drop + Use Asyncio Streams (#29904)

* use non blocking socket + raise on errno 110

* pylint fixes

* comment for errno 110

* [ServiceBus] pyamqp exception parity (#30020)

* unskip tests

* test passing uamqp.TransportType

* make sbreceived messages picklable

* edge case sb message batch creation test

* remove accidental additions

* add sb client tests

* add invalid custom endpoint tests

* update pyamqp invalid custom endpoint error

* add test_errors back to folder

* add more tests

* lint

* fix unskipped async test

* kashif comments

* fix asyncio pickling for <3.11

* lint

* unpickle clients

* remove receiver/uamqp message from received message pickling

* annas comments

* update version + changelog

* update amqp transport kind check in message

* changelog + update to stable

* pull main again

* update readme/typing

* test session set_state None

---------

Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>
Co-authored-by: l0lawrence <llawrence@microsoft.com>
Co-authored-by: Kashif Khan <361477+kashifkhan@users.noreply.github.com>
Co-authored-by: Azure SDK Bot <53356347+azure-sdk@users.noreply.github.com>
Co-authored-by: Paul Van Eck <paulvaneck@microsoft.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.

None yet

4 participants