Skip to content

Conversation

@vishalbollu
Copy link
Contributor

closes #1209


checklist:

  • run make test and make lint
  • test manually (i.e. build/push all images, restart operator, and re-deploy APIs)


def store_metrics_locally(self, status_code, total_time):
status_code_series = int(status_code / 100)
try:
Copy link
Member

Choose a reason for hiding this comment

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

The try/except block isn't required for the following 2 instructions:

self.metrics_file_lock.acquire()
status_code_series = int(status_code / 100)

The 2 above will not fail. The only method that might fail though is self.increment_counter_file. So we could have something like:

try:
    status_code_file_name = f"/mnt/workspace/{os.getpid()}.{status_code_series}XX"
    self.increment_counter_file(status_code_file_name, 1)
    request_time_file = f"/mnt/workspace/{os.getpid()}.request_time"
    self.increment_counter_file(request_time_file, total_time)
finally:
    self.metrics_file_lock.release()

We could also decide to take the 2 string assignments out of the try block. I think this could be a little cleaner:

try:
    self.increment_counter_file(status_code_file_name, 1)
    self.increment_counter_file(request_time_file, total_time)
finally:
    self.metrics_file_lock.release()

@vishalbollu vishalbollu merged commit 705b838 into master Sep 24, 2020
@vishalbollu vishalbollu deleted the add-locks-local-api-metric-recording branch September 24, 2020 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Verify accuracy of request metrics for a local deployment using multiple threads

4 participants