Zpages example#16
Conversation
Merge data aggregator changes
jajanet
left a comment
jajanet
left a comment
There was a problem hiding this comment.
Tiny additions/clarifications
| // Change the name of the running span and end it | ||
| running_span->UpdateName("examplespan2"); | ||
| running_span->End(); |
There was a problem hiding this comment.
What's the purpose of changing a span name, specifically for demonstrating the zPages interface?
There was a problem hiding this comment.
I just want to show that while a span is running the name change does not reflect in zPages only after completion does it show up.
There was a problem hiding this comment.
This could be confusing for the user and not look simple, since I don't imagine users having practical reasons for doing this. The name change will also be instantaneous so users will likely not notice the it, and seeing the code will be confusing.
There was a problem hiding this comment.
I will put a comment in the code
| auto running_span2 = tracer->StartSpan("examplespan2"); | ||
|
|
||
| // Create a completed span every second till user stops the loop | ||
| std::cout << "Presss CTRL+C to stop...\n"; |
There was a problem hiding this comment.
Maybe add to the cout statement that a span is generated every second here would be a good idea? Also, directing the user to the right host:post url could be beneficial.
There was a problem hiding this comment.
I did add a comment right at the top with the url.
There was a problem hiding this comment.
Yes, I mean in the cout statement could be also a good place maybe?
| // Create a span of each type(running, completed and error) | ||
| auto running_span = tracer->StartSpan("examplespan"); | ||
| tracer->StartSpan("examplespan")->End(); | ||
| tracer->StartSpan("examplespan")->SetStatus( |
There was a problem hiding this comment.
Do you think it would be a good idea to demonstrate adding attributes and events to spans, since zPages currently/will show these too?
There was a problem hiding this comment.
I think it's fine for now just to have a basic example. In the app if the user wants he can add attributes.
There was a problem hiding this comment.
I see users adding attributes is standard in these OC/OT apps, and seeing how it's rendered in zPages would be a key way to demonstrate how both of those work while still being basic. I imagine only an additional couple lines max would be needed.
There was a problem hiding this comment.
I added attributes and it shows up but events I don't think is sent down to the span data did you check this?
There was a problem hiding this comment.
Yep, it's not implemented because events (which now have their own attrtibutes and links) are relatively new. Showing events is under stretch goals on the TODO doc, but could be added. Sergey also said it was fine to leave out for now
There was a problem hiding this comment.
Ok I left it out I added attributes and few more output statements to make things more clear
7 participants
No description provided.