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

[sqlserver] remove warning message leaking password strings #3053

Merged
merged 1 commit into from
Dec 1, 2016

Conversation

masci
Copy link
Contributor

@masci masci commented Dec 1, 2016

What does this PR do?

Remove a log.warning call outputting a string containing connection password in case of errors. The log operation was redundant anyways since the exception is correctly and safely propagated.

@@ -329,7 +329,6 @@ def open_db_connections(self, instance):
except Exception as e:
cx = "%s - %s" % (host, database)
message = "Unable to connect to SQL Server for instance %s." % cx
self.log.warning("%s Exception: %s", message, e)
Copy link
Member

@truthbk truthbk Dec 1, 2016

Choose a reason for hiding this comment

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

Was it the exception that contained the connection string, and leaked the password? If so we could leave out the exception, and just print out the cx? I think this integration is multi-instance.... Disregard.

Copy link
Member

@truthbk truthbk left a comment

Choose a reason for hiding this comment

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

I just saw the exception logging was redundant. My bad! 👍

@masci masci merged commit c82dda7 into master Dec 1, 2016
@masci masci deleted the massi/fix-sqlserver-logs branch December 1, 2016 15:32
@olivielpeau olivielpeau added this to the 5.10.2 milestone Dec 6, 2016
degemer added a commit that referenced this pull request Dec 21, 2016
* master: (53 commits)
  [nginx] Update example config
  [service_discovery] Add a Zookeeper service discovery implementation.
  [aggregator] if sample rate is bad, fix it but still parse tags. (#3073)
  [yarn] whitelist authorized application_tags
  Alex poe/update jmx with refresh beans (#3068)
  [config] Fix `_is_affirmative` when passed argument is `None` (#3063)
  Send all configured tags with process checks. (#2976)
  fix flake8 errors
  [flare] ignore whitespace before proxy credentials
  [core] add a switch to disable profiling, but still use developer mode (#2898)
  [tests] allow tests to use the additional_checksd parameter (#3056)
  [service_discovery][jmx] trying to pick-up JMX changes with SD. (#3010)
  [install_script] Make `dd-agent` group of `datadog.conf` (#3036)
  [postgres] Allow disable postgresql.database_size (#3035)
  [core] Fixes IndexError for process lookup (#3043)
  remove warning message leaking password strings (#3053)
  trap psutil.NoSuchProcess exception (#3052)
  Fix grammar and casing in exception text (#3050)
  allow override of kubelet host with KUBERNETES_KUBELET_HOST env var
  [service discovery] properly handle config reload for removed containers
  ...
@masci masci modified the milestones: 5.10.2, 5.12.0 Jan 24, 2017
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