Add option to dynamically determine what APIs to query for metrics (#…

…10815)

* Add test data and limit_reqs endpoint

* Add support for new endpoints in check

* Update metadata with new metrics

* add more tests for tags

* Add more tests and http limit conns endpoint

* Add tests for all plus api versions

* Reformat code and fix typos

* Remove unneeded mappings

* Remove unneeded mapping and test fixture

* Properly handle metrics that should be counts

* Change more metrics to count

* Fix short names of metrics

* Update style and add assertion for all count metrics

* Update conf.example with new description

* Update test folder structure and add mapping for plus api endpoints

* Assert metadata in all tests

* Add missing count metrics, assert metrics covered, and refactor tests

* Remove slashes in directories

* Begin _get_enabled_apis to dynamically determine enabled apis

* Add fixtures

* Comple _get_enabled_endpoints and add more fixtures and test

* Add config models and option

* Add more tests for only_query_enabled_endpoints

* Update config models and remove conflicts

* Update endpoint mapping and fix mock return values

* Add comments and more logging

* Add more log lines and assert them

* Don't mention stream disabled when only_query_enabled_endpoints is enabled

* Add another test for edge cases

* Use more correct url for test

* Apply suggestions from code review

Co-authored-by: Jorie Helwig <jorie.helwig@datadoghq.com>

* Fix style

* Make endpoints a set

* Use dd_run_check

* Remove unused caplog

* Update nginx/assets/configuration/spec.yaml

Co-authored-by: Christine Chen <ChristineTChen@users.noreply.github.com>

* Sync example config

* Use urljoin

Co-authored-by: Jorie Helwig <jorie.helwig@datadoghq.com>
Co-authored-by: Christine Chen <ChristineTChen@users.noreply.github.com>

nginx/assets/configuration/spec.yaml

@@ -50,6 +50,15 @@ files:
       value:
         type: integer
         display_default: 2
+    - name: only_query_enabled_endpoints
+      description: |
+        Enable this option if you would like the check to determine which API endpoints
+        to query based on your NGINX Plus configuration.
+      value:
+        type: boolean
+        example: true
+        display_default: false
+      enabled: true
     - name: use_vts
       description: |
         Set this option to true if you are using the nginx vhost_traffic_status module.

nginx/datadog_checks/nginx/config_models/defaults.py

@@ -104,6 +104,10 @@ def instance_ntlm_domain(field, value):
     return get_default_field_value(field, value)
 
 
+def instance_only_query_enabled_endpoints(field, value):
+    return False
+
+
 def instance_password(field, value):
     return get_default_field_value(field, value)
 

nginx/datadog_checks/nginx/config_models/instance.py

@@ -56,6 +56,7 @@ class Config:
     min_collection_interval: Optional[float]
     nginx_status_url: str
     ntlm_domain: Optional[str]
+    only_query_enabled_endpoints: Optional[bool]
     password: Optional[str]
     persist_connections: Optional[bool]
     plus_api_version: Optional[int]

nginx/datadog_checks/nginx/data/conf.yaml.example

@@ -82,6 +82,12 @@ instances:
     #
     # plus_api_version: <PLUS_API_VERSION>
 
+    ## @param only_query_enabled_endpoints - boolean - optional - default: false
+    ## Enable this option if you would like the check to determine which API endpoints
+    ## to query based on your NGINX Plus configuration.
+    #
+    only_query_enabled_endpoints: true
+
     ## @param use_vts - boolean - optional - default: false
     ## Set this option to true if you are using the nginx vhost_traffic_status module.
     ## If you are using VTS, set the nginx_status_url to something like http://localhost/nginx_stats/format/json.

nginx/datadog_checks/nginx/nginx.py

@@ -7,7 +7,7 @@
 
 import simplejson as json
 from six import PY3, iteritems, text_type
-from six.moves.urllib.parse import urlparse
+from six.moves.urllib.parse import urljoin, urlparse
 
 from datadog_checks.base import AgentCheck, ConfigurationError, to_native_string
 from datadog_checks.base.utils.time import get_timestamp
@@ -52,6 +52,7 @@ def __init__(self, name, init_config, instances):
         self.url = self.instance.get('nginx_status_url')
         self.use_plus_api = self.instance.get("use_plus_api", False)
         self.use_plus_api_stream = self.instance.get("use_plus_api_stream", True)
+        self.only_query_enabled_endpoints = self.instance.get("only_query_enabled_endpoints", False)
         self.plus_api_version = str(self.instance.get("plus_api_version", 2))
         self.use_vts = self.instance.get('use_vts', False)
 
@@ -90,14 +91,75 @@ def check(self, _):
             except Exception as e:
                 self.log.error('Could not submit metric: %s: %s', repr(row), e)
 
+    def _get_enabled_endpoints(self):
+        """
+        Dynamically determines which NGINX endpoints are enabled and Datadog supports getting metrics from
+        by querying the NGINX APIs that list availabled endpoints. If an error is encountered,
+        then it falls back to query all of the known endpoints available in the given NGINX Plus version.
+        """
+        available_endpoints = set()
+
+        base_url = urljoin(self.url + "/", self.plus_api_version)
+        http_url = urljoin(self.url + "/", self.plus_api_version + "/http")
+        stream_url = urljoin(self.url + "/", self.plus_api_version + "/stream")
+
+        try:
+            self.log.debug("Querying base API url: %s", base_url)
+            r = self._perform_request(base_url)
+            r.raise_for_status()
+            available_endpoints = set(r.json())
+            http_avail = "http" in available_endpoints
+            stream_avail = "stream" in available_endpoints
+
+            if http_avail:
+                self.log.debug("Querying http API url: %s", http_url)
+                r = self._perform_request(http_url)
+                r.raise_for_status()
+                endpoints = set(r.json())
+                http_endpoints = {'http/{}'.format(endpoint) for endpoint in endpoints}
+                available_endpoints = available_endpoints.union(http_endpoints)
+
+            if self.use_plus_api_stream and stream_avail:
+                self.log.debug("Querying stream API url: %s", stream_url)
+                r = self._perform_request(stream_url)
+                r.raise_for_status()
+                endpoints = set(r.json())
+                stream_endpoints = {'stream/{}'.format(endpoint) for endpoint in endpoints}
+                available_endpoints = available_endpoints.union(stream_endpoints)
+
+            self.log.debug("Available endpoints are %s", available_endpoints)
+
+            supported_endpoints = self._supported_endpoints(available_endpoints)
+            self.log.debug("Supported endpoints are %s", supported_endpoints)
+            return chain(iteritems(supported_endpoints))
+        except Exception as e:
+            self.log.warning(
+                "Could not determine available endpoints from the API, "
+                "falling back to monitor all endpoints supported in nginx version %s, %s",
+                self.plus_api_version,
+                str(e),
+            )
+            return self._get_all_plus_api_endpoints()
+
+    def _supported_endpoints(self, available_endpoints):
+        """
+        Returns the endpoints that are both supported by this NGINX instance, and
+        that the integration supports collecting metrics from
+        """
+        return {
+            endpoint: nest for endpoint, nest in self._get_all_plus_api_endpoints() if endpoint in available_endpoints
+        }
+
     def collect_plus_metrics(self):
         metrics = []
         self._perform_service_check('{}/{}'.format(self.url, self.plus_api_version))
 
         # These are all the endpoints we have to call to get the same data as we did with the old API
         # since we can't get everything in one place anymore.
 
-        plus_api_chain_list = self._get_all_plus_api_endpoints()
+        plus_api_chain_list = (
+            self._get_enabled_endpoints() if self.only_query_enabled_endpoints else self._get_all_plus_api_endpoints()
+        )
 
         for endpoint, nest in plus_api_chain_list:
             response = self._get_plus_api_data(endpoint, nest)
@@ -186,6 +248,10 @@ def _nest_payload(self, keys, payload):
         return {keys[0]: self._nest_payload(keys[1:], payload)}
 
     def _get_plus_api_endpoints(self, use_stream=False):
+        """
+        Returns all of either stream or default endpoints that the integration supports
+        collecting metrics from based on the Plus API version
+        """
         endpoints = iteritems({})
 
         available_plus_endpoints = PLUS_API_STREAM_ENDPOINTS if use_stream else PLUS_API_ENDPOINTS
@@ -196,6 +262,9 @@ def _get_plus_api_endpoints(self, use_stream=False):
         return endpoints
 
     def _get_all_plus_api_endpoints(self):
+        """
+        Returns endpoints that the integration supports collecting metrics from based on the Plus API version
+        """
         endpoints = self._get_plus_api_endpoints()
 
         if self.use_plus_api_stream:
@@ -214,7 +283,7 @@ def _get_plus_api_data(self, endpoint, nest):
             r = self._perform_request(url)
             payload = self._nest_payload(nest, r.json())
         except Exception as e:
-            if endpoint in PLUS_API_STREAM_ENDPOINTS.values():
+            if not self.only_query_enabled_endpoints and endpoint in PLUS_API_STREAM_ENDPOINTS.values():
                 self.log.warning("Stream may not be initialized. Error querying %s metrics at %s: %s", endpoint, url, e)
             else:
                 self.log.exception("Error querying %s metrics at %s: %s", endpoint, url, e)

nginx/tests/conftest.py

@@ -45,6 +45,26 @@ def instance():
     return copy.deepcopy(INSTANCE)
 
 
+@pytest.fixture(scope='session')
+def instance_plus_v7():
+    base_instance = copy.deepcopy(INSTANCE)
+    base_instance['nginx_status_url'] = 'http://localhost:8080/api'
+    base_instance['use_plus_api'] = True
+    base_instance['use_plus_api_stream'] = True
+    base_instance['plus_api_version'] = 7
+    return base_instance
+
+
+@pytest.fixture(scope='session')
+def instance_plus_v7_no_stream():
+    base_instance = copy.deepcopy(INSTANCE)
+    base_instance['nginx_status_url'] = 'http://localhost:8080/api'
+    base_instance['use_plus_api'] = True
+    base_instance['use_plus_api_stream'] = False
+    base_instance['plus_api_version'] = 7
+    return base_instance
+
+
 @pytest.fixture
 def instance_ssl():
     return {

nginx/tests/fixtures/v1/plus_api.json

@@ -0,0 +1 @@
+["nginx","processes","connections","slabs","http","resolvers","ssl"]

nginx/tests/fixtures/v1/plus_api_http.json

@@ -0,0 +1 @@
+["requests","server_zones","caches","keyvals","upstreams"]

nginx/tests/fixtures/v1/plus_api_stream.json

@@ -0,0 +1 @@
+[]

nginx/tests/fixtures/v7/plus_api.json

@@ -0,0 +1 @@
+["nginx","processes","connections","slabs","http","stream","resolvers","ssl"]

nginx/tests/fixtures/v7/plus_api_http.json

@@ -0,0 +1 @@
+["requests","server_zones","location_zones","caches","limit_conns","limit_reqs","keyvals","upstreams"]

nginx/tests/fixtures/v7/plus_api_stream.json

@@ -0,0 +1 @@
+["server_zones","limit_conns","keyvals","zone_sync","upstreams"]