Skip to content

Conversation

@HadhemiDD
Copy link
Contributor

What does this PR do?

Motivation

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Add the qa/skip-qa label if the PR doesn't need to be tested during QA.
  • If you need to backport this PR to another branch, you can add the backport/<branch-name> label to the PR and it will automatically open a backport PR once this one is merged

@datadog-agent-integrations-bot datadog-agent-integrations-bot bot added documentation release qa/skip-qa Automatically skip this PR for the next QA labels Nov 5, 2025
@codecov
Copy link

codecov bot commented Nov 6, 2025

Codecov Report

❌ Patch coverage is 60.55046% with 43 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.97%. Comparing base (5d1d456) to head (a308cc4).
⚠️ Report is 61 commits behind head on master.

Additional details and impacted files
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@HadhemiDD HadhemiDD marked this pull request as ready for review November 7, 2025 13:06
@HadhemiDD HadhemiDD requested review from a team as code owners November 7, 2025 13:07
@HadhemiDD HadhemiDD changed the title [WIP] n8n integration N8N integration Nov 7, 2025
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

urseberry
urseberry previously approved these changes Nov 7, 2025
Copy link
Contributor

@urseberry urseberry left a comment

Choose a reason for hiding this comment

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

I accidentally selected approve instead of request changes earlier.

Co-authored-by: Ursula Chen <58821586+urseberry@users.noreply.github.com>
@temporal-github-worker-1 temporal-github-worker-1 bot dismissed urseberry’s stale review November 10, 2025 09:33

Review from urseberry is dismissed. Related teams and files:

  • documentation
    • n8n/README.md
Co-authored-by: Ursula Chen <58821586+urseberry@users.noreply.github.com>
@github-actions
Copy link

github-actions bot commented Nov 10, 2025

⚠️ The qa/skip-qa label has been added with shippable changes

The following files, which will be shipped with the agent, were modified in this PR and
the qa/skip-qa label has been added.

You can ignore this if you are sure the changes in this PR do not require QA. Otherwise, consider removing the label.

List of modified files that will be shipped with the agent
n8n/datadog_checks/n8n/__about__.py
n8n/datadog_checks/n8n/__init__.py
n8n/datadog_checks/n8n/check.py
n8n/datadog_checks/n8n/config_models/__init__.py
n8n/datadog_checks/n8n/config_models/defaults.py
n8n/datadog_checks/n8n/config_models/instance.py
n8n/datadog_checks/n8n/config_models/shared.py
n8n/datadog_checks/n8n/config_models/validators.py
n8n/datadog_checks/n8n/data/conf.yaml.example
n8n/datadog_checks/n8n/metrics.py
n8n/changelog.d/21835.added
n8n/pyproject.toml
n8n/hatch.toml

HadhemiDD and others added 6 commits November 10, 2025 10:34
Co-authored-by: Ursula Chen <58821586+urseberry@users.noreply.github.com>
Co-authored-by: Ursula Chen <58821586+urseberry@users.noreply.github.com>
@steveny91
Copy link
Contributor

steveny91 commented Nov 18, 2025

So a few things here that we should clean up:

  • Counter metrics on the raw side we'll need to remove the _total otherwise it won't match
  • On the datadog side, it will come appended with the .count suffix. We should update that in the metadata.
  • Your current unit test has a e2e test in it. It uses the dd_agent_check fixture that doesn't work in unit tests. Chances are it's being skipped.
  • Your e2e doesn't assert metrics. The e2e looking test in the test_unit won't run because we derive the test from the filename so it won't look in that file for a e2e test.

The combination of point 3 and 4 is causing the tests to pass. I think you should either assert metrics as a unit test and mock the http response or you move metric checking to the e2e test.

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.

4 participants