firecracker-microvm / firecracker

Secure and fast microVMs for serverless computing.
http://firecracker-microvm.io
Apache License 2.0
24.7k stars 1.73k forks source link

Use component defined metrics #1759

Open andreeaflorescu opened 4 years ago

andreeaflorescu commented 4 years ago

Design doc and more details here: github.com/andreeaflorescu/metrics-proposal.

This is particularly important for devices, so that we can break the dependency on the Firecracker specific logger implementation. This can be achieved by having devices define their own metrics and take a dependency only on a Metric trait that can be defined in vmm-sys-util.

alindima commented 2 years ago

I believe this was solved by using the *Events traits, like SerialEvents and RtcEvents

andreeaflorescu commented 2 years ago

This is not resolved by using SerialEvents and RtcEvents, but that was indeed a good start.

The proposal was about restructuring how we are defining metrics to fix a few problems:

  1. Remove lazy_static, and move to a design where metrics are defined as an object on the Vmm. For the Firecracker case though we might need to have an App that contains both Vmm and the Api since both can use metrics, and should be able to have access to them.
  2. Be able to run tests in parallel by not relying on a central metric object. If metrics are defined per component, then each unit test can initialize their own metric object, and thus the metrics are local to the test instead of being globally defined.

Here is the proposal, I see the link from the issue is no longer valid: https://github.com/andreeaflorescu/metrics-proposal

This approach can also be used to fix the problem that you are mentioning in #2893

alindima commented 2 years ago

Good point.

This approach can also be used to fix the problem that you are mentioning in https://github.com/firecracker-microvm/firecracker/pull/2893

In order to fully fix the thread-safety issues, multiple other issues need to be solved. Created a related issue: https://github.com/firecracker-microvm/firecracker/issues/2899