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

Review and enhance tracing support for email service (Ruby) #55

Closed
7 of 10 tasks
cartersocha opened this issue May 30, 2022 · 5 comments
Closed
7 of 10 tasks

Review and enhance tracing support for email service (Ruby) #55

cartersocha opened this issue May 30, 2022 · 5 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@cartersocha
Copy link
Contributor

cartersocha commented May 30, 2022

The following is a list of requirements that we need to evaluate before declaring v1 for trace telemetry. These requirements are across the entire application; Not all services will meet all requirements. Determine the relevant features for this service.

  • Automatic Instrumentation is being used where appropriate.
  • Library instrumentation is being used if automatic instrumentation is unavailable.
  • Services extend automatic instrumentation.
    • New attributes/events attached to existing spans.
    • New spans are being created from existing spans.
  • Automatic and Manual Context Propagation are being demonstrated.
  • Telemetry is not being sampled upfront.
  • Telemetry is not being filtered upfront.
  • Baggage is being set and read appropriately (i.e., baggage must be explicitly set as attributes on spans)
  • Update the centralized tracing doc

Referencing: #42

Service: https://github.com/open-telemetry/opentelemetry-demo-webstore/blob/main/src/emailservice/README.md

Ruby dependent on this item: #37

@cartersocha cartersocha added enhancement New feature or request help wanted Extra attention is needed required-for-release labels May 30, 2022
@cartersocha cartersocha changed the title Review and enhance tracing support for product catalog service (Python / Ruby) Review and enhance tracing support for email service (Python / Ruby) May 30, 2022
@cartersocha cartersocha changed the title Review and enhance tracing support for email service (Python / Ruby) Review and enhance tracing support for email service (Ruby) Jun 17, 2022
@cartersocha cartersocha added good first issue Good for newcomers and removed required-for-release labels Jul 14, 2022
@ahayworth
Copy link
Contributor

ahayworth commented Jul 27, 2022

@cartersocha since I can't check off the boxes myself, here's what could be updated right away:

  • Automatic Instrumentation is being used where appropriate.
    • The demo installs automatic instrumentation for Sinatra in the configuration block. Because the ruby app is so simple, there's really nothing else to instrument automatically. 😄
  • Library instrumentation is being used if automatic instrumentation is unavailable.
    • We have automatic instrumentation available, so we should satisfy this condition. I'm not sure what "Library instrumentation" is in this case anyways.

I had some clarifying questions on a few items:

  • Telemetry is not being sampled upfront.
    • Can you clarify what you mean by "sampled"? In the ruby SIG, we use the term "sampled" the way the SDK spec uses it - to mean "this span will be exported."
    • The way that the SDK uses this term in combination with this TODO item makes me think the requirement is that the ruby app should not be exporting spans by default?
    • That's a little confusing to me, and so I'm curious what you mean. On the other hand, if you actually mean "the ruby app should be exporting all spans," then I think we're fine.
    • The ruby SDK installs a "parent-based, always-on" sampler by default. This sampler will always choose to record the span, unless the parent span says otherwise. Since we aren't manually changing the sampler anywhere, that behavior will be what the ruby app exhibits.
    • This is also the default behavior according to the SDK spec.
  • Telemetry is not being filtered upfront.
    • Unclear exactly what you mean by this, but we aren't doing any kind of additional filtering that I am aware of in the ruby SDK.
  • Automatic and Manual Context Propagation are being demonstrated.
    • We aren't calling out to any other service from the ruby app (it's a "leaf" in the tree), so I'm not sure how best to demonstrate additional context propagation.
    • Do you have any advice on where we should demonstrate that?

The remaining items are:

  • baggage
  • updating the centralized tracing doc

Since there's not much remaining that I don't have questions on, I'll wait to get a little clarification on the rest of the TODO items before I begin work. If you want to assign the issue to me, please do so! 😄

@ahayworth
Copy link
Contributor

It occurs to me that one interesting way to make the ruby service exhibit some of the desired behaviors would be to over-complicate it with a job queue. 🤣

...I think that actually might be a good idea. Are there any concerns with that? Conceptually it would be over-complicated, but the actual code to implement it would be rather simple and straightforward. We could more easily demonstrate context and baggage propagation in that way.

Curious what you think.

@cartersocha
Copy link
Contributor Author

Good questions. I'd suggest posting the queue question in the slack channel to get a broader discussion. Sounds interesting to me

@cartersocha
Copy link
Contributor Author

@austinlparker, @puckpuck, what are yalls thoughts on our context propagation requirement. Should we simplify to just creating a new span based off an existing span? (inside service context prop). The instrumentation libraries already have automatic context propagation covered in my mind & connect the services

@cartersocha
Copy link
Contributor Author

Tracing is code complete

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
No open projects
Development

No branches or pull requests

2 participants