Use local vars, not instance vars, in metricsHandler #10

dmazin · 2024-05-24T12:44:09Z

When Falcon starts up, it creates an individual instance of metricsHandler for each metric type:

def falcon_app():
    api = falcon.API()
    api.add_route("/health", metricsHandler(config, metrics_type="health"))

The same metricsHandler instance is used for every /health request.

This, by itself, is not an issue. However, because the instance stores state (such as IP/hostname) in instances variables, these can get clobbered if we process multiple requests simultaneously.

By instance variables I mean usage of self:

class metricsHandler:
    ...
    def on_get(self, req, resp):
        self.target = req.get_param("target")

Here is you can reproduce this.
First, let's add some logging to show the issue.

diff --git a/collector.py b/collector.py
index 32d29d9..e183ddc 100644
--- a/collector.py
+++ b/collector.py
@@ -20,6 +20,8 @@ class RedfishMetricsCollector(object):
        self.target = target
        self.host = host

+       logging.info(f"**** Initializing RedfishMetricsCollector for hostname {self.host} and IP {self.target}")
+
        self._username = usr
        self._password = pwd

diff --git a/handler.py b/handler.py
index b035449..1627ffc 100644
--- a/handler.py
+++ b/handler.py
@@ -33,6 +33,8 @@ class metricsHandler:
        self.metrics_type = metrics_type

    def on_get(self, req, resp):
+       logging.info(f"**** The id of this metricsHandler instance is {id(self)}")
+
        self.target = req.get_param("target")
        if not self.target:
            logging.error("No target parameter provided!")
@@ -87,6 +89,7 @@ class metricsHandler:
        logging.debug(f"{self.target}: Using user {usr}")

+      logging.info(f"**** Scraping {self.host}, the client asked for {req.get_param('target')}")
        with RedfishMetricsCollector(
            self._config,
            target = self.target,

I am going to use the following bash script to quickly make two requests against the exporter:

#!/bin/bash
idracs=("foo" "bar")
for idrac in "${idracs[@]}"; do
    curl "http://localhost:9229/health?target=${idrac}&job=redfish" -o "${idrac}_stats.txt" &
done

Now, let’s observe the logs:

handler.py:36 INFO  *** The id of this metricsHandler instance is 131570941555444
handler.py:36 INFO  *** The id of this metricsHandler instance is 131570941555444
handler.py:94 INFO  *** Scraping foo; the client asked for foo
handler.py:94 INFO  *** Scraping foo; the client asked for bar
collector.py:23 INFO  *** Initializing RedfishMetricsCollector for hostname foo and IP 10.0.0.3.
collector.py:23 INFO  *** Initializing RedfishMetricsCollector for hostname foo and IP 10.0.0.3.

We see a few things:
• Lines 1,2: Indeed, the very same metricsHandler instance processes both requests (this is evidenced by the id being identical). Again, this isn’t an issue unto itself; just saying it’s the same instance so we can’t use self safely.
• Lines 3,4: You can see that it scrapes foo even though the client asked for bar
• Lines 5,6: From collector.py, you can see that the collector is initialized for foo and IP 10.0.0.3 twice even though we had asked for bar on the second request.

fixes #9

BerndKue · 2024-12-10T17:01:44Z

Thanks for your PR.
This is fixed in the meantime. I also saw the issue but struggled for a while to figure out the root cause.
It was exactly as you describe in the instantiation of the metric Handler.

Please let me know if you also see it fixed in the latest version . If not please reopen the PR and I will try to merge things together.

Use local vars, not instance vars, in metricsHandler

c77322f

BerndKue closed this Dec 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use local vars, not instance vars, in metricsHandler #10

Use local vars, not instance vars, in metricsHandler #10

dmazin commented May 24, 2024

BerndKue commented Dec 10, 2024

Use local vars, not instance vars, in metricsHandler #10

Use local vars, not instance vars, in metricsHandler #10

Conversation

dmazin commented May 24, 2024

BerndKue commented Dec 10, 2024