solace-samples-python icon indicating copy to clipboard operation
solace-samples-python copied to clipboard

SOL-64939: Added sampler for OTel

Open TrentDaniel opened this issue 11 months ago • 6 comments

TrentDaniel avatar Mar 28 '24 15:03 TrentDaniel

Hey @TrentDaniel - thanks for taking a stab at this! I have a couple of comments/questions

  1. The python howtos usually have a @staticmethod def run(): that explicitly calls the howto functions in this sampler. See for example how_to_publish_with_backpressure.py
  2. When doing so, and to follow the same conventions across the different API (so far only the Go API has samplers for otel), could you please publish on topic solace/samples/otel-tracing
  3. In the Go API, we had separate samples for publisher and receiver. While I think it can be compact to have a processor sample that publishes and receives the message all in one, we will need to add that in the name of the file or as a comment explaining that. What are your thoughts? I'm indifferent about having separate samplers for publisher and subscriber tho I can see having it in one file would be less duplication and better for the flow.
  4. There exists a package that enforces otel semantic conventions for Go (https://pkg.go.dev/go.opentelemetry.io/[email protected]/semconv/v1.17.0) - is there something similar for python that we should be using? I found this https://pypi.org/project/opentelemetry-semantic-conventions/ but i'm not sure if this the conventional or best practice way to use otel with Python. Just a thought

Overall, I did not run this locally with a separate sample but it would be nice to have a runnable method. And if this is just meant to be code snippets on how to use that I would like to propose we have a sample in the patterns that is accompanied with this howto to show a runnable example

TamimiGitHub avatar Apr 01 '24 18:04 TamimiGitHub

And another question, which might be outside the scope of this PR, when we import packages from the opentel library where and how do we configure the connection to the collector if we want to send application level spans? This code sampler is only involved with configuring the context from the production which will then be propagated through the broker towards the consumer to be extracted

TamimiGitHub avatar Apr 01 '24 18:04 TamimiGitHub

Pending release of solace_otel lib to be published on pypi

TamimiGitHub avatar Apr 01 '24 20:04 TamimiGitHub

And another question, which might be outside the scope of this PR, when we import packages from the opentel library where and how do we configure the connection to the collector if we want to send application level spans? This code sampler is only involved with configuring the context from the production which will then be propagated through the broker towards the consumer to be extracted

I believe this kind of configuration is indeed outside the scope of this PR. Presumably the configuration would be similar, however I'm not familiar with the otelcollector configuration outside of what I've done for integration testing. In a future commit I'll include a small section which shows how to configure the collector for sending and receiving through SMF, so maybe that will have some of the answers you're looking for.

TrentDaniel avatar Apr 03 '24 15:04 TrentDaniel

@TamimiGitHub , just pushed changes in response to PR feedback, let me know if you have any other concerns

TrentDaniel avatar Apr 03 '24 16:04 TrentDaniel

@TamimiGitHub , do you have any more change requests?

TrentDaniel avatar Apr 11 '24 13:04 TrentDaniel

@TamimiGitHub , This PR has been open for a while, do you have any more change requests??

TrentDaniel avatar Jul 12 '24 15:07 TrentDaniel

Hey @TrentDaniel ! I went over the changes and the last comments, I dont have any other requests! Will merge 👍

TamimiGitHub avatar Jul 15 '24 10:07 TamimiGitHub