report language version to plausible - #16733
Open
titusfortner wants to merge 8 commits into
Open
Conversation
Contributor
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||||||||||||||
Contributor
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||
Contributor
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
diemol
approved these changes
Dec 16, 2025
titusfortner
force-pushed
the
lang_version
branch
from
December 16, 2025 18:42
27f55ff to
d441159
Compare
nvborisenko
requested changes
Dec 17, 2025
nvborisenko
left a comment
Member
There was a problem hiding this comment.
Please elaborate motivation, and most likely we propose good solution in .NET. CC @RenderMichael
| { | ||
| StringBuilder argsBuilder = new StringBuilder(arguments); | ||
| argsBuilder.Append(" --language-binding csharp"); | ||
| #if NET8_0_OR_GREATER |
Member
There was a problem hiding this comment.
Original motivation was:
how many people using the latest version of Selenium
This code doesn't include the version of Selenium binding.
Member
Author
There was a problem hiding this comment.
Selenium Manager sets the Selenium version since the versions are in sync: https://github.com/SeleniumHQ/selenium/blob/selenium-4.39.0/rust/src/main.rs#L227
Member
There was a problem hiding this comment.
Plausible analytics are here: https://plausible.io/manager.selenium.dev
navigate to custom properties at the bottom to see specifics.
The reason for the PR is specifically Java, if 50% of our users are still on Java 11, then we aren't going to want to require a move to Java 17, etc
For .NET this means seeing how many people are still using .NET Framework vs NET 6/7 vs NET 8/9/10
Seems reasonable, let's think more about it.
Member
Author
There was a problem hiding this comment.
what is your concern?
Member
There was a problem hiding this comment.
OK, it is not about Selenium version, accepted. I propose to see the final output for all bindings and align the format, I mean python 3.14 or PyThoN_3_14. What is our preference?
Interesting, do we really want to see TFM (net8.0, netstandard2.0)? Or runtime version would be enough?..
PS: We can easily implement all variants.
Member
Author
There was a problem hiding this comment.
As far as capturing TFM, I was thinking it would be useful to know if someone is targeting netstandard2, even if they are using Net 8 or something. Not essential, but the info was right there, so I figured we'd capture it and it only 3x the values at most.
Member
Author
There was a problem hiding this comment.
@RenderMichael {Environment.Version.Major} seems sufficient to me (and limits the number of combinations with TFM). Is there something about the minor version that would give us extra insight?
The goal is to provide understanding about how we focus/update support. We changed targets a couple years ago pretty much blind, and if we had this info it would have made it more obvious what the tradeoffs were.
Contributor
There was a problem hiding this comment.
The minor version is relevant for the .NET Standard 2.0 target. For example, there exists .NET Core 3.0 and .NET Core 3.1 (likewise 2.0 and 2.1, but adoption was so low and only from bleeding-edge folks back that it's completely irrelevant).
The much more relevant examples are on .NET Framework. .NET Standard 2.0 covers 4.7, 4.7.1, 4.7.2, 4.8, and 4.8.1 (as well 4.6.2 but Selenium targets that directly) source. Environment.Version.Major will not give us the information we need there. I don't think Version.Minor will either, actually, given what this outputs on 4.7.2.
Contributor
There was a problem hiding this comment.
I found some guidance mentioning that Environment.Version is not the way to go here https://learn.microsoft.com/en-us/dotnet/framework/install/how-to-determine-which-versions-are-installed#the-environmentversion-property
Member
There was a problem hiding this comment.
We changed targets a couple years ago pretty much blind
I disregards. I proposed to change it, still no issues.
Before:
After:
Anyway collecting usage stats will be helpful. Let's discuss it instantly in slack channel.
Member
Author
|
@nvborisenko The reason for the PR is specifically Java, if 50% of our users are still on Java 11, then we aren't going to want to require a move to Java 17, etc For .NET this means seeing how many people are still using .NET Framework vs NET 6/7 vs NET 8/9/10 |
Co-authored-by: Michael Render <render.michael@gmail.com>
titusfortner
commented
Dec 18, 2025
5 tasks
Contributor
PR Summary by QodoReport language binding version in Selenium Manager stats (Plausible)
AI Description
Diagram
High-Level Assessment
Files changed (19)
|
Contributor
Code Review by Qodo
Context used✅ Tickets:
🎫 2.48 doesn't trigger javascript in link's href on click() 🎫 Instance ChroneDriver Error: ConnectFailure✅ Compliance rules (platform):
42 rules 1. Trait language-version methods undocumented
|
Comment on lines
+1609
to
+1617
| fn get_language_binding_version(&self) -> &str { | ||
| self.get_config().language_binding_version.as_str() | ||
| } | ||
|
|
||
| fn set_language_binding_version(&mut self, language_binding_version: String) { | ||
| if !language_binding_version.is_empty() { | ||
| self.get_config_mut().language_binding_version = language_binding_version; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
1. Trait language-version methods undocumented 📘 Rule violation ✧ Quality
New public trait methods get_language_binding_version and set_language_binding_version were added without Rust doc comments, and the setter lacks required # Arguments documentation. This violates the requirement to document public API items with structured doc comments, reducing API clarity for downstream users.
Agent Prompt
## Issue description
New public trait methods were added without Rust `///` documentation, and the setter is missing a structured `# Arguments` section.
## Issue Context
This is part of the public `SeleniumManager` trait API, so it must be documented with structured Rust doc comments.
## Fix Focus Areas
- rust/src/lib.rs[1609-1617]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| pub os: String, | ||
| pub arch: String, | ||
| pub lang: String, | ||
| pub lang_version: String, |
Contributor
There was a problem hiding this comment.
2. lang_version field undocumented 📘 Rule violation ✧ Quality
A new public field lang_version was added to Props without any Rust doc comment (///) describing its meaning. This violates the requirement to document public API items with structured doc comments, making the public data contract unclear.
Agent Prompt
## Issue description
A new public struct field was added without a Rust doc comment explaining what it represents.
## Issue Context
`Props` is in a `pub mod` and has `pub` fields, so changes add/extend public API surface that must be documented.
## Fix Focus Areas
- rust/src/stats.rs[43-51]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| pub os: String, | ||
| pub arch: String, | ||
| pub lang: String, | ||
| pub lang_version: String, |
Contributor
There was a problem hiding this comment.
3. lang_version metadata undocumented 📘 Rule violation ✧ Quality
A new public field lang_version was added to the public Stats metadata struct without any Rust doc comment (///) describing its meaning. This violates the requirement to document public API items with structured doc comments and makes the metadata schema harder to consume correctly.
Agent Prompt
## Issue description
A new public metadata field was added without Rust `///` documentation.
## Issue Context
`Stats` is a public struct in a public module and is serialized/deserialized, so its fields form a public schema that must be documented.
## Fix Focus Areas
- rust/src/metadata.rs[28-38]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| pub force_browser_download: bool, | ||
| pub avoid_browser_download: bool, | ||
| pub language_binding: String, | ||
| pub language_binding_version: String, |
Contributor
There was a problem hiding this comment.
4. language_binding_version undocumented 📘 Rule violation ✧ Quality
A new public field language_binding_version was added to ManagerConfig without any Rust doc comment (///) explaining its purpose/format. This violates the requirement to document public API items with structured doc comments and reduces maintainability of this public configuration surface.
Agent Prompt
## Issue description
A new public configuration field was introduced without Rust `///` documentation.
## Issue Context
`ManagerConfig` is a public struct in a public module, so new fields expand the public API and must be documented.
## Fix Focus Areas
- rust/src/config.rs[50-72]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| pub os: String, | ||
| pub arch: String, | ||
| pub lang: String, | ||
| pub lang_version: String, |
Contributor
There was a problem hiding this comment.
5. Metadata cache incompatible 🐞 Bug ☼ Reliability
rust/src/metadata.rs adds a new required Stats.lang_version field without a serde default, so older se-metadata.json files will fail deserialization and get_metadata() will drop the entire cache. That can force online lookups and can fail outright in offline mode when driver/browser versions are expected to be served from cached metadata.
Agent Prompt
## Issue description
A new required JSON field (`Stats.lang_version`) was added to the metadata schema, but deserialization does not provide a default. Existing metadata files created by older Selenium Manager versions (without `lang_version`) will fail `serde_json` deserialization, and the current error handling replaces the entire metadata with an empty one.
## Issue Context
The metadata cache is used for TTL-based reuse of discovered browser/driver versions. When it is dropped, Selenium Manager may require online requests again, and in offline mode this can cause failures.
## Fix Focus Areas
- rust/src/metadata.rs[28-38]
- rust/src/metadata.rs[83-104]
- rust/src/chrome.rs[289-310]
## Suggested fix
Make the new field backward-compatible by providing a serde default (or making it optional):
- Option A (simplest):
- Add `#[serde(default)]` to `lang_version: String` in `Stats`.
- Option B:
- Change to `pub lang_version: Option<String>` with `#[serde(default)]`, and treat `None` as "" when comparing.
After the change, older metadata will continue to parse and preserve cached driver/browser entries, while newer versions will still write/read `lang_version`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Note: this PR includes the code from #16736 to get the tests to pass
🔗 Related Issues
Implements #16731
💥 What does this PR do?
Each language obtains version to send to selenium manager
Selenium Manager reports to Plausible
🔧 Implementation Notes
The only nonstandard one is that .NET reports both the target framework and the Runtime Version:
🔄 Types of changes
PR Type
Enhancement
Description
Add language version reporting to Selenium Manager across all bindings
Java reports runtime feature version via Runtime.version().feature()
.NET reports target framework and runtime version with conditional compilation
JavaScript reports Node.js version via process.versions.node
Python reports major.minor version via sys.version_info
Ruby reports version via RUBY_VERSION constant
Rust infrastructure updated to parse and forward language-version argument
Diagram Walkthrough
File Walkthrough
10 files
Add Java runtime version reportingAdd .NET framework and runtime version reportingAdd Node.js version reportingAdd Python version reportingAdd Ruby version reportingAdd language_binding_version field to configAdd getter/setter for language binding versionAdd language-version CLI argument parsingAdd lang_version to stats metadata structuresAdd lang_version field to Props struct