Eventhub tracing #7153

lmazuel · 2019-09-09T23:28:36Z

EventHub part of #6597

AutorestCI · 2019-09-09T23:32:42Z

(message created by the CI based on PR content)

Installation instruction

Package azure-eventhubs

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

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@eventhub_tracing#egg=azure-eventhubs&subdirectory=azure-eventhubs"

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

git checkout eventhub_tracing
pip install -e ./azure-eventhubs

Or build a wheel file to distribute for testing:

git checkout eventhub_tracing
pip wheel --no-deps ./azure-eventhubs

Package azure-eventhubs-checkpointstoreblob-aio

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

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@eventhub_tracing#egg=azure-eventhubs-checkpointstoreblob-aio&subdirectory=azure-eventhubs-checkpointstoreblob-aio"

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

git checkout eventhub_tracing
pip install -e ./azure-eventhubs-checkpointstoreblob-aio

Or build a wheel file to distribute for testing:

git checkout eventhub_tracing
pip wheel --no-deps ./azure-eventhubs-checkpointstoreblob-aio

Direct download

Your files can be directly downloaded here:

sdk/eventhub/azure-eventhubs/azure/eventhub/aio/consumer_async.py

sdk/eventhub/azure-eventhubs/azure/eventhub/aio/eventprocessor/event_processor.py

sdk/eventhub/azure-eventhubs/azure/eventhub/aio/producer_async.py

sdk/eventhub/azure-eventhubs/azure/eventhub/consumer.py

sdk/eventhub/azure-eventhubs/azure/eventhub/producer.py

sdk/eventhub/azure-eventhubs/azure/eventhub/aio/eventprocessor/event_processor.py

lmolkova · 2019-09-16T18:11:16Z

sdk/eventhub/azure-eventhubs/azure/eventhub/aio/producer_async.py

+            message_span = child.span(name="Azure.EventHubs.message")
+            message_span.start()
+            app_prop = dict(message.application_properties)
+            app_prop.setdefault(b"Diagnostic-Id", message_span.get_trace_parent().encode('ascii'))


what if message was already instrumented (in previous send try or by user)?

setdefault will not owerwrite the item if it is already set. We could avoid the cost of encoding by doing an in check, however.

I don't expect this encode to take a lot, since the string is 0-9A-Z and the encoder is ascii. That will let mostly cast it "as-is" under the hood.
With the in, in case we do need to assign it, we will do another lookup to find the right spot. Without the in, we indeed ascii-encode for nothing sometimes, but if we need to insert we know already where it is.

… error message, but this will fix the analyze error

YijunXieMS

EventHubConsumer of both sync and aio has next / anext. They should also be tracked? Users can receive evetns by:

for event in consumer:  # sync EventHubConsumer 
    pass  #  do something

or

async for event in consumer: # async EventHubConsumer
    pass  #  do something

lmazuel · 2019-09-25T21:22:43Z

@YijunXieMS it has been decided in arch meeting that "receive" should not be traced.

YijunXieMS · 2019-09-26T01:30:36Z

/azp run python - eventhubs - tests

azure-pipelines · 2019-09-26T01:30:44Z

Azure Pipelines successfully started running 1 pipeline(s).

sdk/eventhub/azure-eventhubs/azure/eventhub/client_abstract.py

sdk/eventhub/azure-eventhubs/azure/eventhub/producer.py

YijunXieMS · 2019-09-26T18:32:56Z

sdk/eventhub/azure-eventhubs/azure/eventhub/producer.py

+                self._client._add_span_request_attributes(child)  # pylint: disable=protected-access
+                self._send_event_data_with_retry(timeout=timeout)
+        else:
+            self._send_event_data_with_retry(timeout=timeout)


both if and else has this statement. How about removing the else block and pull the same statement in "if" out of "if"?

YijunXieMS · 2019-09-27T18:32:47Z

/azp run python - eventhubs - tests

azure-pipelines · 2019-09-27T18:32:57Z

Azure Pipelines successfully started running 1 pipeline(s).

YijunXieMS

LGTM

…into base_to_properties * 'master' of https://github.com/Azure/azure-sdk-for-python: Eventhub tracing (Azure#7153) [devtools-cognitiveservices] temp fix to allow passing cog key in headers for azure-core (Azure#7471) adds back create mgmt client (Azure#7470) Update SDK Tools Consumption (Azure#7148) [AutoPR] peering/resource-manager (Azure#6923) Key Vault: making poller return Certificate or CertificateOperation (Azure#7349) Patch os.environ in Identity mock tests (Azure#7452) Unskip urllib3 related test (Azure#7442)

* Experimentation on tracing and EventHubs * Continue to let the initial excp raise * Naive direct tracing implementation * Update Kind in EventHub to generic one * Use contextmanager in EventHub * Remove opencensus specific import * Fix possible AttributeError * Remove parent concept * Remove receive tracing code * Don't execute tracing on message if no tracing loaded * Try to re-order dev dep for CI * Fix EH plugin dev deps * Add azure-core to azure-eventhub * Share req * ChangeLog * EH extension is ok with b4 * Install blob SDK for EH extension * pylint * fix dev req * dep fix * the override had <. the setup actually defines <=. need to update the error message, but this will fix the analyze error * Tracing message from receive iterators as well * Producer simplification * Simplify eventprocessor * Consider batch size

lmazuel added Event Hubs Client This issue points to a problem in the data-plane of the library. labels Sep 9, 2019

lmazuel force-pushed the eventhub_tracing branch from 2fa2f3e to b1a4ae9 Compare September 10, 2019 16:11

joshfree requested a review from lmolkova September 10, 2019 21:15

lmazuel force-pushed the eventhub_tracing branch 3 times, most recently from 710ecbf to 6a51f46 Compare September 12, 2019 23:21

lmazuel requested a review from johanste September 12, 2019 23:32

lmazuel marked this pull request as ready for review September 12, 2019 23:32

lmazuel requested review from annatisch, bryevdv, rakshith91, xiangyan99, YijunXieMS, yunhaoling and zezha-msft as code owners September 12, 2019 23:32

lmazuel changed the base branch from master to tracing_update September 12, 2019 23:32

lmazuel removed request for annatisch, rakshith91 and zezha-msft September 12, 2019 23:33

lmolkova reviewed Sep 12, 2019

View reviewed changes

lmazuel force-pushed the eventhub_tracing branch from fde07a4 to 3ced2ce Compare September 13, 2019 22:50

lmolkova reviewed Sep 16, 2019

View reviewed changes

lmazuel force-pushed the eventhub_tracing branch from 6e0e7ca to dedae18 Compare September 19, 2019 19:01

lmolkova approved these changes Sep 19, 2019

View reviewed changes

lmazuel changed the base branch from tracing_update to master September 23, 2019 21:18

lmazuel force-pushed the eventhub_tracing branch from dedae18 to 57dddb5 Compare September 23, 2019 21:18

lmazuel added 2 commits September 24, 2019 13:00

Experimentation on tracing and EventHubs

40c9d8f

Continue to let the initial excp raise

93dc908

lmazuel added 3 commits September 24, 2019 13:00

Try to re-order dev dep for CI

443cdc8

Fix EH plugin dev deps

83d7128

Add azure-core to azure-eventhub

78b78ab

lmazuel force-pushed the eventhub_tracing branch from 22ec850 to 78b78ab Compare September 24, 2019 20:01

lmazuel added 8 commits September 24, 2019 13:07

Share req

22b78c2

ChangeLog

82a39ef

EH extension is ok with b4

db31f6b

Merge branch 'master' into eventhub_tracing

d3433c3

Install blob SDK for EH extension

632a8d8

pylint

c4c4fd1

fix dev req

9fb2039

dep fix

2f4df0a

yunhaoling mentioned this pull request Sep 25, 2019

[EventHubs]Add distributed tracing support #6315

Closed

the override had <. the setup actually defines <=. need to update the…

af4acd7

… error message, but this will fix the analyze error

YijunXieMS reviewed Sep 25, 2019

View reviewed changes

lmazuel added 3 commits September 25, 2019 15:49

Tracing message from receive iterators as well

ce2948a

Producer simplification

bd2da67

Simplify eventprocessor

b025231

YijunXieMS reviewed Sep 26, 2019

View reviewed changes

Consider batch size

32ce0e8

YijunXieMS approved these changes Sep 27, 2019

View reviewed changes

YijunXieMS merged commit dc7dfb0 into master Sep 27, 2019

lmazuel deleted the eventhub_tracing branch October 2, 2019 15:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Eventhub tracing #7153

Eventhub tracing #7153

lmazuel commented Sep 9, 2019 •

edited

Loading

AutorestCI commented Sep 9, 2019 •

edited

Loading

lmolkova Sep 16, 2019

johanste Sep 16, 2019

lmazuel Sep 19, 2019 •

edited

Loading

YijunXieMS left a comment

lmazuel commented Sep 25, 2019 •

edited

Loading

YijunXieMS commented Sep 26, 2019

azure-pipelines bot commented Sep 26, 2019

YijunXieMS Sep 26, 2019

YijunXieMS commented Sep 27, 2019

azure-pipelines bot commented Sep 27, 2019

YijunXieMS left a comment

Eventhub tracing #7153

Eventhub tracing #7153

Conversation

lmazuel commented Sep 9, 2019 • edited Loading

AutorestCI commented Sep 9, 2019 • edited Loading

Installation instruction

Package azure-eventhubs

Package azure-eventhubs-checkpointstoreblob-aio

Direct download

lmolkova Sep 16, 2019

Choose a reason for hiding this comment

johanste Sep 16, 2019

Choose a reason for hiding this comment

lmazuel Sep 19, 2019 • edited Loading

Choose a reason for hiding this comment

YijunXieMS left a comment

Choose a reason for hiding this comment

lmazuel commented Sep 25, 2019 • edited Loading

YijunXieMS commented Sep 26, 2019

azure-pipelines bot commented Sep 26, 2019

YijunXieMS Sep 26, 2019

Choose a reason for hiding this comment

YijunXieMS commented Sep 27, 2019

azure-pipelines bot commented Sep 27, 2019

YijunXieMS left a comment

Choose a reason for hiding this comment

lmazuel commented Sep 9, 2019 •

edited

Loading

AutorestCI commented Sep 9, 2019 •

edited

Loading

lmazuel Sep 19, 2019 •

edited

Loading

lmazuel commented Sep 25, 2019 •

edited

Loading