Fix: make Orchestrator metrics singleton

[eero-t]

Description

Make Orchestrator metrics singleton, so that even if applications instantiate multiple Orchestrators, "megaservice_*" metrics collect data from all of them.

This is intended as proper fix for the #1280 workaround. When only CI tests created multiple Orchestrator instances, changing metric prefix was fine, but now that applications (e.g. DocSum) do that too, that's not the case any more.

(Another option would be to add arguments for passing Orchestrator instance names as metric prefixes, to name and differentiate metrics for each Orchestrator instance. However, that would have needed changes in 4 OPEA projects instead of just this one, and dashboards & benchmarks would then need to hard-code those per-application prefixes.)

Additionally, this also change timings to be done based on time.monotonic().

Issues

• Adding Orchestrator instances to applications changes their metric names, depending on in which order those instances are created, and it distorts the whole application metric values. This breaks dashboards and benchmarks relying on those metric names
• System clock updates could mess timings based on time.time()

Type of change

• Bug fix (non-breaking change which fixes an issue)

Dependencies

n/a.

Tests

• Metric testing added to CI TestServiceOrchestratorStreaming test passes
• Manual testing to verify that:
  • Metrics are OK for ChatQnA
  • DocSum megaservice metrics use only single prefix after this PR

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines	Coverage Δ
comps/cores/mega/orchestrator.py	91.39% <100.00%> (+0.52%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[eero-t]

Rebased to main, did testing and removed draft status.

Metrics work. There's something funky with DocSum LLM-uservice HuggingFace API endpoint handling, when it is stressed more (it seems to try to connect HF site for every query and eventually fail), but I don't see how it could be related to these changes.

[xiguiw]

@eero-t
Is it possible to create a test case in CI test to test this?

[eero-t]

@eero-t Is it possible to create a test case in CI test to test this?

@xiguiw I'll try to modify (streaming) ServiceOrchestrator test to check also metrics.

[eero-t]

CI fails on something completely unrelated to changes in this PR:

chatqna-gaudi-nginx-server Error manifest for opea/nginx:comps not found: manifest unknown: manifest unknown
Error response from daemon: manifest for opea/nginx:comps not found: manifest unknown: manifest unknown

[eero-t]

@eero-t Is it possible to create a test case in CI test to test this?

@xiguiw Ok, I've expanded (streaming) ServiceOrchestrator test to cover also checking of its streaming metrics.

It showed odd metrics values, which did not happen with real apps, only with test code.

=> It was ServiceOrchestrator bug in propagating first token status, which is now fixed.

---

CI "example-test" continues to fail, but it's completely unrelated to this PR, and should not block merging:

[ retrieval ] HTTP status is not 200. Received status was 000
Traceback (most recent call last):
  File "/usr/local/lib/python3.11/site-packages/llama_index/core/utils.py", line 63, in __init__
    nltk.data.find("corpora/stopwords")
  File "/usr/local/lib/python3.11/site-packages/nltk/data.py", line 579, in find
    raise LookupError(resource_not_found)
LookupError: 
**********************************************************************
  Resource stopwords not found.
  Please use the NLTK Downloader to obtain the resource:

  >>> import nltk
  >>> nltk.download('stopwords')

[eero-t]

https://github.com/opea-project/GenAIComps/pull/1340/files and https://github.com/opea-project/GenAIComps/pull/1343/files should fix the CI issue:

Resource stopwords not found.
Please use the NLTK Downloader to obtain the resource

But I think it may be cleaner if I submit commits for the metrics test and fix to issues it revealed, as a separate PR.

[eero-t]

I split test code addition and fixes to #1348, it should be merged first.

[eero-t]

Streaming metrics test PR (#1348) was merged, so rebased to main. Codecov reports now 100% coverage for the changes.

@xiguiw, @Spycsh OK to merge this fix?

[Spycsh]

LGTM

[eero-t]

Rebased to main, added commit changing time.time() calls to time.monotonic(), for more correct timings.

[eero-t]

@lvliang-intel, @ftian1, @letonghan, @XinyaoWa Could someone of you provide the 2nd review for this fix, so it can be merged?