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

Use lazy logging format #5398

Merged
merged 40 commits into from
Jan 7, 2020
Merged

Use lazy logging format #5398

merged 40 commits into from
Jan 7, 2020

Conversation

AlexandreYang
Copy link
Member

@AlexandreYang AlexandreYang commented Jan 6, 2020

What does this PR do?

Use lazy logging format for ALL integrations in integrations-core.

Will enforce the rule in another PR. ddev is used for integrations extras/internal, so, we need to fix them too, should be faster though since there is less code.

CI results with logging linter enabled: #5400 (PR All is green)

Motivation

Better logging.

Additional Notes

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached

@codecov
Copy link

codecov bot commented Jan 6, 2020

Codecov Report

Merging #5398 into master will decrease coverage by 0.89%.
The diff coverage is 66.66%.

Impacted Files Coverage Δ
kubelet/datadog_checks/kubelet/prometheus.py 80.87% <ø> (ø) ⬆️
process/datadog_checks/process/process.py 75.16% <ø> (ø) ⬆️
directory/datadog_checks/directory/directory.py 88.88% <0%> (ø) ⬆️
openstack/datadog_checks/openstack/openstack.py 40.66% <0%> (ø)
kubelet/datadog_checks/kubelet/cadvisor.py 77.06% <0%> (ø) ⬆️
postfix/datadog_checks/postfix/postfix.py 57.14% <100%> (ø) ⬆️
...master/datadog_checks/mesos_master/mesos_master.py 85.82% <100%> (ø) ⬆️
kong/datadog_checks/kong/kong.py 84.31% <100%> (ø) ⬆️
disk/datadog_checks/disk/disk.py 86.69% <100%> (ø) ⬆️
...troller/datadog_checks/openstack_controller/api.py 80.05% <33.33%> (ø) ⬆️
... and 374 more

@codecov
Copy link

codecov bot commented Jan 6, 2020

Codecov Report

Merging #5398 into master will decrease coverage by 0.92%.
The diff coverage is 65%.

Impacted Files Coverage Δ
spark/datadog_checks/spark/spark.py 82.44% <ø> (ø) ⬆️
mapreduce/datadog_checks/mapreduce/mapreduce.py 70.53% <ø> (ø) ⬆️
kubelet/datadog_checks/kubelet/prometheus.py 80.87% <ø> (ø) ⬆️
oracle/datadog_checks/oracle/oracle.py 88.05% <ø> (ø) ⬆️
process/datadog_checks/process/process.py 75.16% <ø> (ø) ⬆️
ibm_mq/datadog_checks/ibm_mq/ibm_mq.py 88.57% <0%> (ø) ⬆️
directory/datadog_checks/directory/directory.py 88.88% <0%> (ø) ⬆️
openstack/datadog_checks/openstack/openstack.py 40.66% <0%> (ø)
ibm_mq/datadog_checks/ibm_mq/config.py 84.93% <0%> (ø) ⬆️
kubelet/datadog_checks/kubelet/cadvisor.py 77.06% <0%> (ø) ⬆️
... and 149 more

@AlexandreYang AlexandreYang force-pushed the alex/use_lazy_logging_format branch from 894abb5 to 7f54c7d Compare January 6, 2020 19:10
Copy link
Contributor

@ofek ofek left a comment

Choose a reason for hiding this comment

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

)
'`collect_status_metrics_by_host` is enabled but no host info could be extracted from HAProxy '
'stats endpoint for %s',
service,
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: service service was unassigned at this point. That's why service, _, status = host_status has been moved upward.

@@ -411,7 +411,7 @@ def _check_slowlog(self, instance, custom_tags):
max_slow_entries = int(conn.config_get(MAX_SLOW_ENTRIES_KEY)[MAX_SLOW_ENTRIES_KEY])
if max_slow_entries > DEFAULT_MAX_SLOW_ENTRIES:
self.warning(
"Redis {0} is higher than {1}. Defaulting to {1}. "
"Redis {0} is higher than {1}. Defaulting to {1}. " # noqa: G001
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: this case is somewhat legit, hence noqa

except socket.gaierror as e:
err_code, message = e.args
if err_code == socket.EAI_NODATA or err_code == socket.EAI_NONAME:
raise socket.error('Unable to resolve host, check your DNS: {}'.format(message))
raise socket.error('Unable to resolve host, check your DNS: {}'.format(message)) # noqa: G
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: seems to be a bug in linter, not sure why it's catching this case (will have a look at the plugin code later)

@AlexandreYang AlexandreYang merged commit 14ef6bd into master Jan 7, 2020
@AlexandreYang AlexandreYang deleted the alex/use_lazy_logging_format branch January 7, 2020 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment