gemmellr commented on a change in pull request #319:
URL: https://github.com/apache/qpid-proton/pull/319#discussion_r692955572
##########
File path: cpp/src/tracing.cpp
##########
@@ -168,32 +168,25 @@ void on_message_start_span(message &message, delivery &d)
{
trace::Scope scope = get_tracer()->WithActiveSpan(span);
- if (delivery_state == "unknown")
- {
- span->AddEvent("Delivery state is unknown");
- }
- if (delivery_state == "Received")
- {
- span->AddEvent("Delivery is received");
- }
-
tag_span[tag_in_binary] = span;
}
-void on_message_end_span(delivery &d) {
+void on_message_end_span(delivery &d, tracker &track) {
binary tag = d.tag();
nostd::shared_ptr<trace::Span> span = tag_span[tag];
- // std::cout << "Inside on_message_end_span, before end" << std::endl;
+ transfer tt(track);
+ std::string delivery_state = to_string(tt.state());
+ span->AddEvent("delivery state: "+ delivery_state);
span->End();
- // std::cout << "Inside on_message_end_span, after end" << std::endl;
}
-void on_settled_span(delivery &d) {
+void on_settled_span(delivery &d, tracker &track) {
binary tag = d.tag();
nostd::shared_ptr<trace::Span> span = tag_span[tag];
- // std::cout << "Inside on_settled_span, before end" << std::endl;
+ transfer tt(track);
+ std::string delivery_state = to_string(tt.state());
+ span->AddEvent("delivery state: "+ delivery_state);
Review comment:
Similarly, I might omit this one if not set. Though its less clear given
this point does mark the end of the message handling from the senders
perspective as the other peer is done with the message settled (whereas with
the on_message_end_span ending, that completing just means the handler
returned, it doesnt necessarily mean the processing is done).
@ssorj thoughts?
##########
File path: cpp/src/tracing.cpp
##########
@@ -168,32 +168,25 @@ void on_message_start_span(message &message, delivery &d)
{
trace::Scope scope = get_tracer()->WithActiveSpan(span);
- if (delivery_state == "unknown")
- {
- span->AddEvent("Delivery state is unknown");
- }
- if (delivery_state == "Received")
- {
- span->AddEvent("Delivery is received");
- }
-
tag_span[tag_in_binary] = span;
}
-void on_message_end_span(delivery &d) {
+void on_message_end_span(delivery &d, tracker &track) {
binary tag = d.tag();
nostd::shared_ptr<trace::Span> span = tag_span[tag];
- // std::cout << "Inside on_message_end_span, before end" << std::endl;
+ transfer tt(track);
+ std::string delivery_state = to_string(tt.state());
+ span->AddEvent("delivery state: "+ delivery_state);
Review comment:
I would possibly omit doing this if there isnt any delivery state yet
(which is a possibility, should the application or the client not have finished
processing the message and set one yet)
##########
File path: .github/workflows/build.yml
##########
@@ -60,9 +66,59 @@ jobs:
if: runner.os == 'macOS'
run: |
brew install libuv swig pkgconfig jsoncpp
+ - name: Checkout curl repo (for jaeger)
Review comment:
As mentioned on IRC, I expect there is a package that can be installed
to provide whatever e.g development headers are needed by the other build
(until such time later that it too can just be used via packages).
For now this works so its good to concentrate on the other actual tracing
aspects first and come back to it later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]