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

Add new lag in seconds metric #11861

Merged
merged 14 commits into from
May 10, 2022
Merged

Conversation

piochelepiotr
Copy link
Contributor

What does this PR do?

Adds a new metric for the kafka_consumer integration: lag in seconds.
It's a best effort metric, but I think it's still really valuable. Lag in seconds is much more usable than lag in messages (currently available metric).

Motivation

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

@piochelepiotr piochelepiotr force-pushed the piotr-wolski/add-lag-seconds-kafka-check branch 2 times, most recently from c770c88 to b00a110 Compare May 3, 2022 08:27
@piochelepiotr piochelepiotr marked this pull request as ready for review May 3, 2022 08:34
@piochelepiotr piochelepiotr requested a review from a team as a code owner May 3, 2022 08:34
@piochelepiotr piochelepiotr requested a review from hithwen May 3, 2022 08:35
@piochelepiotr piochelepiotr force-pushed the piotr-wolski/add-lag-seconds-kafka-check branch from b00a110 to ddb0dd9 Compare May 3, 2022 09:49
@codecov
Copy link

codecov bot commented May 3, 2022

Codecov Report

Merging #11861 (e2f26db) into master (d996317) will increase coverage by 0.00%.
The diff coverage is 89.88%.

Flag Coverage Δ
kafka_consumer 84.38% <89.88%> (+1.72%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@github-actions
Copy link

github-actions bot commented May 3, 2022

The validations job has failed; please review the Files changed tab for possible suggestions to resolve.

@piochelepiotr piochelepiotr force-pushed the piotr-wolski/add-lag-seconds-kafka-check branch from 5408f67 to 6380152 Compare May 4, 2022 09:24
@github-actions
Copy link

github-actions bot commented May 4, 2022

The validations job has failed; please review the Files changed tab for possible suggestions to resolve.

@piochelepiotr piochelepiotr force-pushed the piotr-wolski/add-lag-seconds-kafka-check branch from ba19cad to 80ccedc Compare May 4, 2022 14:25
@yzhan289 yzhan289 changed the title kafka_consumer: Add lag new metric: lag in seconds Add lag new lag in seconds metric May 6, 2022
yzhan289
yzhan289 previously approved these changes May 6, 2022
@yzhan289 yzhan289 changed the title Add lag new lag in seconds metric Add new lag in seconds metric May 6, 2022
Comment on lines +7 to +17
try:
from datadog_agent import read_persistent_cache, write_persistent_cache
except ImportError:

def write_persistent_cache(key, value):
# type: (str, str) -> None
pass

def read_persistent_cache(key):
# type: (str) -> Optional[str]
return ''
Copy link
Contributor

@hithwen hithwen May 9, 2022

Choose a reason for hiding this comment

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

Why this? The feature has been in the agent for years, the minimun supported base check already has it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. I copied it from another check but you have more context than me on it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you think it's safe to switch to directly import datadog_agent, I'm happy to change it.

@hithwen hithwen merged commit f371924 into master May 10, 2022
@hithwen hithwen deleted the piotr-wolski/add-lag-seconds-kafka-check branch May 10, 2022 15:16
@fanny-jiang fanny-jiang mentioned this pull request Feb 9, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants