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

Feature: Adding tabId to log #409

Merged
merged 4 commits into from
Feb 8, 2024
Merged

Feature: Adding tabId to log #409

merged 4 commits into from
Feb 8, 2024

Conversation

mdiep-cese
Copy link

Logs that are not associated with tab events do not have information about which tab (tab Id) that the activities occurred in. We add tab ID information to all logs to facilitate future capability to parse logs based on the tab.

@Jyyjy Jyyjy self-requested a review February 7, 2024 17:25
Copy link
Contributor

@Jyyjy Jyyjy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we sandbox the tabid functionality to the extension? Tabid is only meaningful when using the plugin and shouldn't be included in main script.

I suggest injecting the tabId field to logs when they are ingested by background.js. You can achieve this by adding sender to the callback handler and inspecting the tab field.

Edit for documentation link:
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/Runtime/onMessage#parameters

@Jyyjy
Copy link
Contributor

Jyyjy commented Feb 7, 2024

Sorry for stepping on your toes here, but it seemed easier to code what I was trying to describe rather than describe it. Let me know if you approve of these changes and feel free to overwrite them.

@mdiep-cese
Copy link
Author

No apology is needed! Thanks for the insight and providing the suggested changes. There's a couple of things I'd like to test and will let you know.

@mdiep-cese
Copy link
Author

I took a look at the change and verify it with the produced logs. Looks good!

@EandrewJones
Copy link
Contributor

Thanks for reviewing Jason.

@Jyyjy Jyyjy merged commit 36b5bb3 into apache:test Feb 8, 2024
2 checks passed
@Jyyjy Jyyjy deleted the feat/tabId branch February 8, 2024 15:05
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

Successfully merging this pull request may close these issues.

3 participants