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

Implement event logger #80

Closed
remusao opened this issue Jan 3, 2019 · 5 comments · Fixed by #251
Closed

Implement event logger #80

remusao opened this issue Jan 3, 2019 · 5 comments · Fixed by #251
Milestone

Comments

@remusao
Copy link
Collaborator

remusao commented Jan 3, 2019

To help debugging, it would be nice to implement an event logger for the adblocker. This would optionally keep track of requests blocked/redirected, exceptions as well as cosmetics injected into the page.

@remusao
Copy link
Collaborator Author

remusao commented Jul 24, 2019

@sentialx @Kikobeats I'm following-up here on the discussion about implementing events which can be use to keep track of blocked requests, etc.

We could have a common mechanism to WebExtensionBlocker, PuppeteerBlocker and ElectronBlocker; to avoid repetition. It could even be implemented in @cliqz/adblocker (core package) in the FiltersEngine class so that all sub-classes benefit from this. I was not 100% sure about the API, and how much information we want to expose. There are multiple events that we could want to keep track of:

  • requests blocked
  • requests redirected
  • requests which could have been blocked but benefited from an exception
  • CSP headers injections
  • events related to cosmetics injections: scripts injected, styles injected

Depending on the project and use-case, you might want these information or not. Maybe a simple counter of "requests altered" is enough. But before implementing such mechanism, I'd like to answer these questions so that we build something solid which can accommodate multiple use-cases.

In terms of implementation details, it could be using some EventEmitter (maybe FiltersEngine could implement this API and define events for each case we are interested in), or using some callbacks. I'd also like to make sure the performance over-head is as small as possible. Ideally, the overhead is zero is no-one is listening to events, and negligible when events are consumed.

@sentialx
Copy link
Contributor

sentialx commented Jul 24, 2019

I think these events:

  • requests blocked
  • requests redirected
  • requests which could have been blocked but benefited from an exception

could be emitted in the FiltersEngine class and these:

  • CSP headers injections
  • events related to cosmetics injections: scripts injected, styles injected

in the abstraction classes.

@remusao
Copy link
Collaborator Author

remusao commented Jul 24, 2019

Could you elaborate on why we should split these?

@sentialx
Copy link
Contributor

These two are dependent on abstraction classes cosmetic filtering and CSP injection implementations which can't be implemented in a common way.

@remusao
Copy link
Collaborator Author

remusao commented Jul 24, 2019

But in a way this is the same thing for request blocking right? FiltersEngine only returns "what should be done", then sub-classes apply (or not) the blocking in a platform-specific way. Same goes for cosmetics. In a way both are problematic because if FiltersEngine emits event for request blocked or cosmetics, but the sub-class decides (for some reason, or because of a bug) to not apply the blocking, then the count will not correspond to reality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants