feat(metrics): use async local storage for metrics

[svozza]

Summary

This PR adds support for using an async context (specifically from the InvokeStore package) in the Metrics utility. This allows users to emit metrics that are isolated specifically to the current lambda invocation, isolated from any other executions.

Changes

• Added specific storage classes for metrics, metadata and dimensions:
  • MetricsStore
  • MetadataStore
  • DimensionStore
• The Metrics class only accesses the data in these stores through this interface and never touches the stored objects directly.
• These storage classes check whether they are running in the InvokeStore context: if they are then the metrics are stored in the current async context, otherwise we fallback to a plain instance wide object as per the current implementation.
• The storage class handle converting the data into the correct format now rather than the Metrics class, e.g., setMetric will check if the metric already exists and handle converting values into an array if the metric is already there. Likewise with setting timestamps.
• Added specific concurrency tests to ensure isolation is working as intended for all three stores and the Metrics class as a whole.

Issue number: closes #4662

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

[dreamorosi]

Very nice work here, I haven't tested it and I won't be able to for a while's but the implementation makes a lot of sense - please if possible run some tests on your machine with the correct setup as we discussed.

Other than that, do you have any idea of whether there's a significant performance impact for this new implementation when used on a Lambda on demand?

[dreamorosi]

Since we added this as dependency of the commons pkg, do we need to declare it here again?

[svozza]

Oh that's a good point, we may not. Let me play around with it.

[svozza]

Maybe showing my ignorance of npm workspaces but if it's used in multiple packages across the workspace, should it not be in the root package.json file rather than commons or metrics or whatever?

[dreamorosi]

The main package.json doesn't get published to npm, only the package ones do. This means it won't be installed when the package gets installed.

In this case, it should be either in both or just in commons, since metrics depends on it.

When present in both, if the dependency version is aligned, it gets deduplicated, so only one copy gets installed.

In this case I was suggesting to bypass this risk entirely and just keep it as dependency for commons, but I don't feel too strongly about it since I don't expect this package to change very often.

[svozza]

Ah yes, I knew I was missing something. Honestly, I'd rather be explicit so you can see what dependencies a package uses when you look at the dependency file so I'd be in favour of keeping it here.

[dreamorosi]

Why are we spreading and creating a new object here?

[svozza]

Because I don't want it to be possible to get a reference to the underlying object and be able to mutate it without going through the store interface.

[svozza]

Very nice work here, I haven't tested it and I won't be able to for a while's but the implementation makes a lot of sense - please if possible run some tests on your machine with the correct setup as we discussed.

Other than that, do you have any idea of whether there's a significant performance impact for this new implementation when used on a Lambda on demand?

Yes, I plan to do some perf testing (vs the original version) in a real lambda function on Monday.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud