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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[馃殌 Feature]: OpenTelementry tracing: be able to link the default tracing with a custom tracing #9943

Closed
llatinov opened this issue Oct 16, 2021 · 9 comments 路 Fixed by #10024

Comments

@llatinov
Copy link
Contributor

Feature and motivation

Be able to link custom OpenTelemetry tracing with the default Selenium grid tracing. See for e.g. https://github.com/llatinov/selenium-observability-java/blob/324b500316be34f5f1426720353d0c4e2e8b5a89/selenium-tests/src/main/java/com/automationrhapsody/observability/TracingWebDriver.java#L81
It will be useful if I am able to pass my Span as a parent span of Selenium's RemoteWebDriver tracing, so I can trace my actions with corresponding grid actions.

Usage example

Implement a custom tracing that monitors the tests. It will be of great help to be able to make the default server/grid tracing a child of the custom tracking. In this case, users can monitor how a click of a button in their tests links to grid performance.

@pujagani
Copy link
Contributor

Thank you for providing the details and helping us understand the feature required. We do have a way to trace a request from the client to the server. As far as I understand, connecting a trace should not be a problem as long as the correct trace headers are present. Please refer to https://www.selenium.dev/documentation/webdriver/remote_webdriver/#beta-2-onwards. Any chance you have tried this out and it did not suffice or function?
If so, we can then ideate on how to get this up and running.

@llatinov
Copy link
Contributor Author

llatinov commented Nov 1, 2021

Hello, I have tried this, and it is working correctly. As you said, it is tracing the request from the client to the server. What I want is a different use case. I am instrumenting my tests, and I have linked them with the application under test, so I can trace the whole setup. Both traces - my custom one and Selenium default one are not connected in any way. What I suggest here is to link both traces, so while tracing the tests and application, users can get additional information on how the grid had performed meanwhile.

@pujagani
Copy link
Contributor

pujagani commented Nov 3, 2021

馃憢 I understand the requirement better after pulling in the project you shared and trying to hook it up with Selenium's observability. Thank you for sharing the repo information and providing the detailed requirement and motivation.
After a few attempts yesterday, the change needed does not seem straightforward. Essentially context propagation is the issue here. Passing a Span and using it to create a child span in the RemoteWebDriver does not seem to be a straightforward option, because we have an internal wrapper around OpenTelemetry to ensure compatibility and ease of changes with the upgrade.
Hence, the only way I see is passing the context will fix it. I have tried to pass the context from the repo to the RemoteWebDriver instance, but even that has not done the trick. It is probably due to how the creation of a span updates it each time and then linking to that updated context becomes tricky. Let me know if you have some insights on the same! I would appreciate any thoughts or suggestions. Thank you!

@llatinov
Copy link
Contributor Author

llatinov commented Nov 3, 2021

I checked the wrapper you have. I also believe passing the context down to the Selenium wrapper might be a good approach. WIll do some experiments and will update the issue.

llatinov added a commit to llatinov/selenium that referenced this issue Nov 4, 2021
Provide OpenTelemetry context from outside,
in order to connect the custom tracing with Selenium traces.
Context is set with: OpenTelemetryTracer.getInstance()
.setOpenTelemetryContext(Context.current().with(span));

Fixes SeleniumHQ#9943
@llatinov
Copy link
Contributor Author

llatinov commented Nov 6, 2021

@pujagani I have attached an experimental branch. I set my context into OpenTelemetryTracer and then Selenium uses it from there. In my code I set the context with: OpenTelemetryTracer.getInstance().setOpenTelemetryContext(Context.current().with(span));

This achieves what I need. Please, take a look and let me know if this can fit into the Selenium architecture concepts. If so, I will add tests and create a PR.

full_traces

@pujagani
Copy link
Contributor

pujagani commented Nov 8, 2021

Thank you for sharing the details and the code. I achieved something similar, but locally I was using the OpenTelemetry APIs rather than the wrapper APIs we have. The next step was for me was to create methods to set the context and get the same context and use it as needed to stitch it all together.

Screenshot 2021-11-08 at 11 02 03 AM

I think your changes achieve what I was intending with my local changes. Appreciate your time and effort on this. Please create a PR. Thank you for your contribution :)

pujagani pushed a commit to llatinov/selenium that referenced this issue Nov 23, 2021
Provide OpenTelemetry context from outside,
in order to connect the custom tracing with Selenium traces.
Context is set with: OpenTelemetryTracer.getInstance()
.setOpenTelemetryContext(Context.current().with(span));

Fixes SeleniumHQ#9943
pujagani added a commit that referenced this issue Nov 23, 2021
鈥racing


Fixes #9943

Co-authored-by: Puja Jagani <puja.jagani93@gmail.com>
@llatinov
Copy link
Contributor Author

@pujagani thanks for merging the PR, I am expecting the next release to try it out. When the next version is released, I believe is good to update the documentation: https://www.selenium.dev/documentation/webdriver/remote_webdriver/ ?

@pujagani
Copy link
Contributor

Thank you for your contribution. Yes, after the next release, it is a good idea to update the documentation regarding the support of integration with custom tracing.

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 25, 2021
elgatov pushed a commit to elgatov/selenium that referenced this issue Jun 27, 2022
鈥racing


Fixes SeleniumHQ#9943

Co-authored-by: Puja Jagani <puja.jagani93@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants