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

[NDM] [Cisco ACI] Improve integration performance #18747

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

zoedt
Copy link
Contributor

@zoedt zoedt commented Oct 1, 2024

What does this PR do?

https://datadoghq.atlassian.net/browse/NDMII-2903

https://datadoghq.atlassian.net/wiki/spaces/II/pages/4169761001/Cisco+ACI+Performance+Investigation

Summary of changes made:

  • Refactored the API call to get the interface statistics (aggregated into call when getting the interface list for a device)
    • Also filtered for only the metrics we were interested in (5min intervals, etc.) to reduce large response size
  • Refactored fabric tests
    • Iteration one, there is still a lot of improvements to be made

Things to note

  • The number of metrics is significantly decreased with the interface stats endpoint refactor
    • Previously, if an interface was 'down' and not sending information, the response will incl. the stats anyways with values all being 0. Now, if the interface is down, no stats will be returned, thus no metrics will be sent - this seems to be desired behavior / follows other NDM integrations, but welcome to feedback!
  • Another ticket will be filed to fix the excessive amount of metrics sent (they end up overwritten)
    • Example: 5+ CPU metrics from one device during one check run, the last one ends up being what actually gets sent to DD

Motivation

Significant check duration is a consistent concern for customers

Additional Notes

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Changelog entries must be created for modifications to shipped code
  • 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

Copy link

codecov bot commented Oct 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.58%. Comparing base (979e3f3) to head (4158d0c).

Additional details and impacted files
Flag Coverage Δ
activemq ?
cassandra ?
cisco_aci 89.58% <100.00%> (-5.48%) ⬇️
hive ?
hivemq ?
ignite ?
jboss_wildfly ?
kafka ?
presto ?
solr ?

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

Copy link

github-actions bot commented Oct 7, 2024

The changelog type changed or removed was used in this Pull Request, so the next release will bump major version. Please make sure this is a breaking change, or use the fixed or added type instead.

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.

1 participant