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

[Feature] Istio Mixer and Mesh endpoints are optional #3875

Merged
merged 12 commits into from
Jun 18, 2019

Conversation

mikekatica
Copy link
Contributor

What does this PR do?

This PR enables the istio integration to be configured with any combination of endpoints, instead of requiring Mesh and Mixer.

Motivation

This PR is a solution for an open issue I have with DD support (internal #227716). This stems from an issue with the integration regarding multiple instances of pilot. If using the flat config file to create the checks in the agent, the kubernetes service is used to access endpoints of the istio components. This is fine if you have one instance of telemetry, galley, and pilot. We have multiple pilot instances, and at any time the kubernetes service only routes to one of them. This is not helpful if we want to sum all metrics across pilot instances. The solution to this problem is A) this PR, and B) use autodiscovery annotations on the pilot (or other istio component) pod to have datadog gather metrics from each discrete instance.

Additional Notes

I admit my pytest experience is not great, so the tests of Pilot only and Galley only instances are just copies of existing tests with some changes.

Review checklist (to be filled by reviewers)

  • 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
  • Feature or bugfix must have tests
  • Git history must be clean
  • If PR adds a configuration option, it must be added to the configuration file.

Michael Katica (CMG-Videa) added 3 commits June 5, 2019 13:22
Incremented version to 2.4.0 and made all configs optional with at least 1 required for integration to succeed.
Updated documentation to reflect changes to the integration.
Configured tests for Pilot only and Galley only instances.
@mikekatica mikekatica requested review from a team as code owners June 5, 2019 19:37
cswatt
cswatt previously approved these changes Jun 5, 2019
Copy link
Contributor

@cswatt cswatt left a comment

Choose a reason for hiding this comment

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

Docs approved.

Fixed an issue where the mesh endpoint wasn't getting properly setup.
Michael Katica (CMG-Videa) added 6 commits June 5, 2019 19:41
Another typo. Fixed.
Added proper iterator for pilot and galley metrics
Increased Agent Requirement version to 2.4.0 from 2.3.0.
Fixed Flake8 style errors
Fixed one missing style change in tests and one in conf.yaml.example
Reformatted some mocks as per recommendations.
Copy link
Contributor

@therve therve left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your patch, it makes a lot of sense. I have some small comments, but otherwise it looks good to me.

istio/CHANGELOG.md Outdated Show resolved Hide resolved
istio/datadog_checks/istio/__about__.py Outdated Show resolved Hide resolved
istio/datadog_checks/istio/istio.py Outdated Show resolved Hide resolved
istio/datadog_checks/istio/istio.py Outdated Show resolved Hide resolved
istio/tests/test_check.py Show resolved Hide resolved
istio/tests/test_check.py Show resolved Hide resolved
requirements-agent-release.txt Outdated Show resolved Hide resolved
Michael Katica (CMG-Videa) added 2 commits June 6, 2019 09:56
Rephrased some comments and removed release related version changes.
Changed one more comment for clarity.
@codecov
Copy link

codecov bot commented Jun 6, 2019

Codecov Report

Merging #3875 into master will increase coverage by 6.08%.
The diff coverage is 95.65%.

@@            Coverage Diff             @@
##           master    #3875      +/-   ##
==========================================
+ Coverage   86.85%   92.93%   +6.08%     
==========================================
  Files         737        4     -733     
  Lines       37971      184   -37787     
  Branches     4418       24    -4394     
==========================================
- Hits        32979      171   -32808     
+ Misses       3832        6    -3826     
+ Partials     1160        7    -1153

1 similar comment
@codecov
Copy link

codecov bot commented Jun 6, 2019

Codecov Report

Merging #3875 into master will increase coverage by 6.08%.
The diff coverage is 95.65%.

@@            Coverage Diff             @@
##           master    #3875      +/-   ##
==========================================
+ Coverage   86.85%   92.93%   +6.08%     
==========================================
  Files         737        4     -733     
  Lines       37971      184   -37787     
  Branches     4418       24    -4394     
==========================================
- Hits        32979      171   -32808     
+ Misses       3832        6    -3826     
+ Partials     1160        7    -1153

Copy link
Contributor

@therve therve left a comment

Choose a reason for hiding this comment

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

Looks great to me. We're in feature freeze for now, but I think we can merge it soon after. I'll let other people have a look too.

Thanks!

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.

None yet

4 participants