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

Specify a Logging module #45

Closed
mjzffr opened this issue Aug 4, 2020 · 6 comments
Closed

Specify a Logging module #45

mjzffr opened this issue Aug 4, 2020 · 6 comments

Comments

@mjzffr
Copy link

mjzffr commented Aug 4, 2020

WebDriver BiDi should have a Logging module to make it easy to monitor messages logged from various sources. This module might be a good candidate for early prototyping.

In practice, we've seen test automation assert that there is no logging to the console at all, or failing if there's any message text that matches a certain pattern, which brings pitfalls with any browser-specific log output.

The protocol should distinguish different kinds log entries and allow filtering based on various criteria to further reduce traffic over the websocket.

One event:

  • logEntryAdded. Some possible parameters:
    • level (String): e.g. info
    • text (String): the log message
    • kind (String): is it a Console API call, a kind of error or warning (CSS, JS, security), a network request?
    • scriptContextId or browsingContextId for which this log entry was added
    • timestamp (number)
    • extra (object): additional optional metadata that applies to different kinds of log entries
      • url: resource URL for which this log entry was added
      • args: Console API call args
      • method: Console API method called
      • lineNumber
      • colNumber
      • requestId: network request ID
      • bootstrapScriptId if the entry originated from a bootstrap script

In addition to being able to subscribe to the logEntryAdded event, we should be able to narrow down the subscription with filters like the kind, a url prefix, scriptContextId, whether it's from a bootstrap script...

Maybe the general subscribe method could have a match parameter to specify such filters along the same lines as the match patterns in the bootstrap-scripts proposal.

@shs96c
Copy link

shs96c commented Aug 10, 2020

This seems focused on console-style logs (that is, not things like network performance), and I like the approach. Logging came up a few times in the f2f sessions while working on the original webdriver spec. There was a discussion in 2013 and a later one in 2017

In addition, the Selenium project has a logging API that is already implemented by the chromedriver. Michael Klepikov gave a talk about how to use this at GTAC in 2013 too. The underlying APIs are here in the selenium source. The key classes being Logs and LogEntry

While these APIs should be event-based in webdriver bidi, there are some basic concepts that might be nice to consider as we add these capabilities to WebDriver bidi:

  • Can select the types of logs to collect (already noted in the original issue)
  • Allows logs from intermediary nodes to be collected

The ability to introspect into more than just the end node can be a really useful feature.

@foolip
Copy link
Member

foolip commented Aug 12, 2020

One kinds of logs that have come up in browser testing is the stdout/stderr of the browser itself, used to get stack traces if there's a crash. Is this something we'd consider in scope for this? cc @stephenmcgruer

@css-meeting-bot
Copy link
Member

The Browser Testing and Tools Working Group just discussed Logging.

The full IRC log of that discussion <AutomatedTester> Topic: Logging
<AutomatedTester> jgraham: One of the value add module proposals is a logging module
<AutomatedTester> ... we know that Selenium have asked this as a priority
<AutomatedTester> ... so it would be good get requirements
<AutomatedTester> github topic: https://github.com//issues/45
<AutomatedTester> ... is this a good enough or do we need to extend it?
<AutomatedTester> ... do we need to get browser internal logs? Intermediary logs?
<AutomatedTester> ... and for clarification network logging is not part of this proposal and we can add it to a newer API
<AutomatedTester> q+
<jgraham> q+
<jgraham> ack AutomatedTester
<AutomatedTester> AutomatedTester: I have a few questions around this around filtering of logging as cloud providers could DDoS clients and then how granular is the data?
<AutomatedTester> ack jgraham
<AutomatedTester> jgraham: the 2nd question : we allow people in the spec currently to handle this in browser context or realm
<jimevans> q+
<AutomatedTester> ...and for the first question for this we don't have a mechanism for this at the moment?
<AutomatedTester> ... and we dont have a precendent in this area to learn from
<AutomatedTester> AutomatedTester: I was thinking of the difference between console.log vs console.error and only getting the latter
<AutomatedTester> q?
<AutomatedTester> jimevans: At a high level here that I hear from selenium users is
<AutomatedTester> ... they want to get console.log between 2 time points
<AutomatedTester> ... and if there are unhandled JS errors, notify me
<jgraham> RRSAgent: make minutes v2
<RRSAgent> I have made the request to generate https://www.w3.org/2020/10/27-webdriver-minutes.html jgraham
<AutomatedTester> ... at a less frequency is I want to get the performance logging that I see in my devtools console
<jgraham> q+
<jgraham> ack jimevans
<AutomatedTester> ... at a bare minimum we need to do the console logs and unhandled exceptions
<AutomatedTester> ... and I agree that there are concerns around chattiness
<AutomatedTester> ... from my experience of using CDP, those 2 things are similar to me
<AutomatedTester> ... it would be good to find out from a driver to get what logging they support
<AutomatedTester> ... and we need to agree on the general shape of the log entries
<AutomatedTester> q?
<cb> q+
<AutomatedTester> jgraham: The performance is in scope for the spec but not high priority
<AutomatedTester> ... since perf is browser specific... it's important but not low hanging fruit
<simonstewart> q+
<jgraham> ack jgraham
<AutomatedTester> ... the main issue that I am hearing is what type of filtering is there
<AutomatedTester> ack cb
<AutomatedTester> cb: from my experience implementing perf tooling it would be hard for us to spec
<AutomatedTester> ... it would be good to have the log entry to be more generic in where it's cominf from
<AutomatedTester> q?
<AutomatedTester> ack simonstewart
<drousso> q+
<AutomatedTester> simonstewart: this is bikeshedding. Having a logging module shouldnt be there. It should be on a JavaScript module
<simonstewart> Or a `Console` module
<AutomatedTester> ... and happy to take this bikeshedding to the proposal later
<jgraham> s/JavaScript/JavaScript or Console/
<AutomatedTester> q?
<AutomatedTester> ack drousso
<AutomatedTester> drousso: as a friendly warning there are always new console messages... and filtering should be done via an event
<AutomatedTester> ... this is how webinspector works
<AutomatedTester> jgraham: luckly this is what is in the proposal but thank you for the warning on new items being added
<AutomatedTester> Resolution: Take proposal in issue and turn into spec prose

@jgraham
Copy link
Member

jgraham commented Oct 27, 2020

So to summarise the discussion, I think there's general agreement that the design here is the right start. There seems to be a preference for the name Console vs Logging. There's also a concern that new types of log events are added frequently, so we should ensure that the design is extensible, and can support vendor-specific event types. There's also a question about how to implement filtering by type or level since some sites can produce many log messages; that might need to be a seperate state-setting command.

@jgraham
Copy link
Member

jgraham commented Nov 17, 2020

#73 has an early cut of this

@jgraham
Copy link
Member

jgraham commented Feb 15, 2021

The initial PR landed, so I'm going to close this even though there's more to be done.

@jgraham jgraham closed this as completed Feb 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants