Skip to content

Conversation

@ddl-kavya
Copy link
Contributor

@ddl-kavya ddl-kavya commented Dec 11, 2025

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting)
  • Refactoring (no functional changes)
  • CI related changes
  • Other... Please describe:

What is the current behavior?

Issue Number: https://dominodatalab.atlassian.net/browse/QE-18127

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@ddl-kavya ddl-kavya requested a review from a team as a code owner December 11, 2025 06:56
@ddl-kavya ddl-kavya requested review from ddl-cedricyoung and juwatanabe and removed request for a team and juwatanabe December 11, 2025 06:56
Copy link
Collaborator

@ddl-cedricyoung ddl-cedricyoung left a comment

Choose a reason for hiding this comment

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

@ddl-kavya

  1. I think this ticket requires a little more comprehensive approach. There is a lot of methods in this file browser_steps.py that don't have logging.
  2. Also I think we're missing logging at a higher level, that is in the set method for config.CONFIG[variable] =
  3. And we should update tests to ensure this logging continues to work.

@ddl-kavya
Copy link
Contributor Author

@ddl-cedricyoung If we add logging at

@ddl-kavya

  1. I think this ticket requires a little more comprehensive approach. There is a lot of methods in this file browser_steps.py that don't have logging.
  2. Also I think we're missing logging at a higher level, that is in the set method for config.CONFIG[variable] =
  3. And we should update tests to ensure this logging continues to work.

@ddl-cedricyoung if we add logging at higher level in the set method, it will log all the config values useful(port,host_address, scenario metadata, hooks, timeouts, browser info) and sensitive(credentials, tokens, API keys, cookies, headers, 2FA secrets). Even if we just capture just useful information it's too much noise in the report.

@ddl-cedricyoung
Copy link
Collaborator

@ddl-cedricyoung if we add logging at higher level in the set method, it will log all the config values useful(port,host_address, scenario metadata, hooks, timeouts, browser info) and sensitive(credentials, tokens, API keys, cookies, headers, 2FA secrets). Even if we just capture just useful information it's too much noise in the repor

@ddl-kavya did you try it? I'm assuming that the secret scrubbing already scrubs out the sensitive info, but I might be mistaken.

@ddl-kavya
Copy link
Contributor Author

ddl-kavya commented Dec 24, 2025

@ddl-cedricyoung Yes, I tried it in my local and it is not scrubbing the sensitive information.
Below is the screenshot from one of the tests. All these config values are logged

image

@ddl-cedricyoung ddl-cedricyoung self-requested a review December 24, 2025 20:02
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.

4 participants