Skip to content

Commit

Permalink
[mongo] skip collStats and indexStats on unauthorized local db collec…
Browse files Browse the repository at this point in the history
…tions (#19244)

* skip collStats and indexStats on unauthorized local db collections

* add changelog
  • Loading branch information
lu-zhengda authored Dec 11, 2024
1 parent 82b68f8 commit bdf67e4
Show file tree
Hide file tree
Showing 9 changed files with 83 additions and 265 deletions.
1 change: 1 addition & 0 deletions mongo/changelog.d/19244.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Skip unauthorized `local` database collections `system.replset`, `replset.election`, and `replset.minvalid` in collection and index stats gathering to avoid permission errors.
12 changes: 12 additions & 0 deletions mongo/datadog_checks/mongo/collectors/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ def __init__(self, check, tags):
self.metrics_to_collect = self.check.metrics_to_collect
self._collection_interval = None
self._collector_key = (self.__class__.__name__,)
self._system_collections_skip_stats = {
"local": frozenset(["system.replset", "replset.election", "replset.minvalid"])
}

def collect(self, api):
"""The main method exposed by the collector classes, needs to be implemented by every subclass.
Expand All @@ -37,6 +40,15 @@ def compatible_with(self, deployment):
"""Whether or not this specific collector is compatible with this specific deployment type."""
raise NotImplementedError()

def should_skip_system_collection(self, coll_name):
"""Whether or not the collection should be skipped because collStats or indexStats
is not authorized to run on certain system collections.
"""
db_name = getattr(self, "db_name", None)
if not db_name or db_name not in self._system_collections_skip_stats:
return False
return coll_name in self._system_collections_skip_stats[db_name]

def _normalize(self, metric_name, submit_method, prefix=None):
"""Replace case-sensitive metric name characters, normalize the metric name,
prefix and suffix according to its type.
Expand Down
4 changes: 4 additions & 0 deletions mongo/datadog_checks/mongo/collectors/coll_stats.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ def _get_collection_stats(self, api, coll_name):
def collect(self, api):
coll_names = self._get_collections(api)
for coll_name in coll_names:
if self.should_skip_system_collection(coll_name):
self.log.debug("Skipping collStats for system collection %s.%s", self.db_name, coll_name)
continue

# Grab the stats from the collection
try:
collection_stats = self._get_collection_stats(api, coll_name)
Expand Down
4 changes: 4 additions & 0 deletions mongo/datadog_checks/mongo/collectors/index_stats.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ def _get_collections(self, api):
def collect(self, api):
coll_names = self._get_collections(api)
for coll_name in coll_names:
if self.should_skip_system_collection(coll_name):
self.log.debug("Skipping indexStats for system collection %s.%s", self.db_name, coll_name)
continue

try:
for stats in api.index_stats(self.db_name, coll_name):
idx_name = stats.get('name', 'unknown')
Expand Down
1 change: 1 addition & 0 deletions mongo/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ def instance_integration_autodiscovery(instance_integration):
instance["database_autodiscovery"] = {
"enabled": True,
}
instance.pop("collections", None)
return instance


Expand Down
24 changes: 24 additions & 0 deletions mongo/tests/fixtures/$collStats-oplog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,24 @@
"localTime": {
"$date": "2024-07-01T20:36:49.358Z"
},
"latencyStats": {
"reads": {
"latency": 13165,
"ops": 10
},
"writes": {
"latency": 8542,
"ops": 1
},
"commands": {
"latency": 0,
"ops": 0
},
"transactions": {
"latency": 0,
"ops": 0
}
},
"storageStats": {
"size": 907806,
"count": 4341,
Expand Down Expand Up @@ -196,6 +214,12 @@
"totalIndexSize": 0,
"indexSizes": {},
"scaleFactor": 1
},
"queryExecStats": {
"collectionScans": {
"total": 81753,
"nonTailable": 81750
}
}
}
]
1 change: 1 addition & 0 deletions mongo/tests/fixtures/list_collection_names-local
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
["oplog.rs", "replset.minvalid"]
5 changes: 4 additions & 1 deletion mongo/tests/mocked_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,10 @@ def command(self, command, *args, **_):
return json.load(f, object_hook=json_util.object_hook)

def list_collection_names(self, session=None, filter=None, comment=None, **kwargs):
with open(os.path.join(HERE, "fixtures", "list_collection_names"), 'r') as f:
filename = f"list_collection_names-{self._db_name}"
if not os.path.exists(os.path.join(HERE, "fixtures", filename)):
filename = "list_collection_names"
with open(os.path.join(HERE, "fixtures", filename), 'r') as f:
return json.load(f)

def aggregate(self, pipeline, session=None, **kwargs):
Expand Down
Loading

0 comments on commit bdf67e4

Please sign in to comment.