Maintenance: Metrics types and Implementation

[niko-achilles]

Summary

High level overview , can be described with details in a time point.

1. Tactics to handle Data Model, that is outside the boundaries of Powertools:

• e.g: EMF Data Model that is used in Metrics.

2. serialize metrics code does additional things besides serialization, it produces as result the state of the EMF Data Model.

• e.g filters , maps , validates and then serializes : result state of the EMF Data Model

3. Concepts of types that are used for StoredMetric in Metrics

• e.g: comparison (old, new Metric), definition of, units of metric

4. Metrics Interface that is implemented by Metrics does not provide hints to a contributor when
   contributor has the intention to extend the functionality of Metrics

• e.g: when extending the interface of a given a method by introducing a new parameter the class that implements the interface does not provide feedback about the new parameter introduced in the method interface.

5. Comments of code in areas of Metrics lack of consistency and a contributor cannot derive a pattern to write comments in code when the intention is to add new functionality with comments in code .

6. Unit Tests of Metrics lack of utilities that can be reused when the intention is to introduce a new test when extending functionality of Metrics

• e.g : prepare phase , create a metric in a namespace , with dimensions, etc. is repeated in tests .

Why is this needed?

High level description of maintenance improvements

1. Tactics to handle Data Model, that is outside the boundaries of Powertools:

• e.g: EMF Data Model that is used in Metrics.

• Why is needed ? purpose:
  It will improve readability and learning for contribution when the EMF Schema as typescript interface or type is provided.
  It will also protect from type/interface explosion , given a contributor can create similar types in the types/Metrics.ts .

• Use case :
  By introducing the high-resolution metrics implementation a new type is introduced in types/Metrics.ts which is imported in Metrics.ts implementation .

https://github.com/awslabs/aws-lambda-powertools-typescript/blob/79a321b199ef51a024dc25b83673baf2eb03de69/packages/metrics/src/types/Metrics.ts#L71-L75

This MetricDefinition type which is in the types/Metrics.ts is referenced in the EmfOutput type

https://github.com/awslabs/aws-lambda-powertools-typescript/blob/79a321b199ef51a024dc25b83673baf2eb03de69/packages/metrics/src/types/Metrics.ts#L17-L27

This is a decision made by understanding a pattern during the implementation. The MetricUnits were referenced in the EmfOutput type, and as result the MetricDefinition type is created .
However the MetricDefinition type has the same meaning as MetricDefinition of EMF Data Model .

The same can happen by combing other properties of Emf Data Model and introduce new type aliases with the same meaning as Emf . That will mean type explosion .

Also a contributor by reading the types/Metrics.ts expects to read about types that support the mechanics of Metrics.ts .
Is the MetricDefinition a type imported in Metrics.ts used in public methods ? Does the MetricDefinition type belong to the types/Metrics.ts ?
Is the mutable EmfOutput type used in Metrics.ts ? As is defined today it is mutable or does the Output mean readonly ? . Does the EmfOutput type belong to the types/Metrics.ts ?

2. serialize metrics code does additional things besides serialization, it produces as result the state of the EMF Data Model.

• e.g filters , maps , validates and then serializes : result state of the EMF Data Model
• why is needed: readability reasons.

The index Type

https://github.com/awslabs/aws-lambda-powertools-typescript/blob/79a321b199ef51a024dc25b83673baf2eb03de69/packages/metrics/src/types/Metrics.ts#L67-L69

And the mapReducer

https://github.com/awslabs/aws-lambda-powertools-typescript/blob/79a321b199ef51a024dc25b83673baf2eb03de69/packages/metrics/src/Metrics.ts#L370

And the return Type EmfOutput that is produced from serialize can be improved for readability and consistency.

3. Concepts of types that are used for StoredMetrics in Metrics

• e.g: comparison (old, new Metric), definition of, units of metric
• Why is needed ? purpose: The isNew Metric is implemented as below . The if statement comparison (not equal) is used for old metrics.
  However the units are implemented with structural typing support (enum and type combination). Can a unit of an old metric be inconsistent ?

https://github.com/awslabs/aws-lambda-powertools-typescript/blob/79a321b199ef51a024dc25b83673baf2eb03de69/packages/metrics/src/Metrics.ts#L477-L480

4. Metrics Interface that is implemented by Metrics does not provide hints to a contributor when
   contributor has the intention to extend the functionality of Metrics

• e.g: when extending the interface of a given a method by introducing a new parameter the class that implements the interface does not provide feedback about the new parameter introduced in the method interface.
• Why is needed ? purpose: guidance to extend the functionality with tight feedback loop when righting tests firsts and then in steps extending the functionality.

5. Comments of code in areas of Metrics lack of consistency and a contributor cannot derive a pattern to write comments in code when the intention is to add new functionality with comments in code .

• Why is needed ? purpose: which guide can be used by the contributor ? tsdoc, jsdoc or markdown that can result in a balance for rendering comments of code in IDE and API docs .
• in vs code some tsdoc or jsdoc attributes are not parsed nice , like {@link }
• should a contributor create comments in code for types ? should the methods that use the types link to the type and be clickable ?
• should the optional parameters have a code comment about the default value ?

6. Unit Tests of Metrics lack of utilities that can be reused when the intention is to introduce a new test when extending functionality of Metrics

• e.g : prepare phase of unit tests, create a new metric, add a metric , set dimensions, etc. is repeated in tests .
• Why is needed ? purpose: in the preparation phase, reuse the creation of metric with good default values, can improve readability and learning for new contributors. Also created metrics with defaults that can parametrized in the act phase of the test can support the creation of good new tests by the contributors.

Which area does this relate to?

Metrics

Solution

• considerations :
  The Emf type could be introduced in a separate artifact Emf.ts
  The Emf type could be treated as given in pure Emf format without introducing inside this type new types created in powertools like the use case by introducing new types with the same meaning , MetricDefiniton.
  Is a property from emf needed in powertools , then it would improve handling the external dependency to the emf data model and improve readability when the property from the emf type is extracted directly from emf.

Does the MetricDefinition type belong to the types/Metrics.ts ?
This type could be defined inside the Metrics.ts implementation or in a location where it is not exported as part of the types of the metrics package .

type MetricDefinition = {
  Name: Emf['_aws']['CloudWatchMetrics'][0]['Metrics'][0]['Name']
  Unit: Emf['_aws']['CloudWatchMetrics'][0]['Metrics'][0]['Unit']
  StorageResolution: Emf['_aws']['CloudWatchMetrics'][0]['Metrics'][0]['StorageResolution']
} 

Additional
Is the mutable EmfOutput type used in Metrics.ts ? As is defined today it is mutable or does the Output mean readonly ?
The type of EmfOutput could be made explicit by refactoring as example to : ReadOnly<Emf> where Emf is the pure Data Model of EMF as defined by the EMF Schema and ReadOnly is a typerscript utility type (immutalble).

Does the ReadOnly<Emf> or EmfOutput type belong to the types/Metrics.ts ?
This type could be defined inside the Metrics.ts implementation or in a location where it is not exported as part of the types of the metrics package .

considerations:
The index Type for Stored Metrics can be refactored to a Record utility Type from Typescript with static structural support at compilation time.

https://github.com/awslabs/aws-lambda-powertools-typescript/blob/79a321b199ef51a024dc25b83673baf2eb03de69/packages/metrics/src/types/Metrics.ts#L67-L69

And the mapReducer can be made more readable with a Record utility type for Stored Metrics, acting in a role of Store that can be used for mapping to an Emf result.

https://github.com/awslabs/aws-lambda-powertools-typescript/blob/79a321b199ef51a024dc25b83673baf2eb03de69/packages/metrics/src/Metrics.ts#L370

And the return Type EmfOutput can be ReadOnly, is produced from serialize, improves readability and consistency.

consideration:
Tests for isNewMetric , comparing old and new mertics . Can a unit of an old metric be inconsistent ?

The isNew Metric is implemented as below . The if statement comparison (not equal) is used for old metrics.
However the units are implemented with structural typing support (enum and type combination).

https://github.com/awslabs/aws-lambda-powertools-typescript/blob/79a321b199ef51a024dc25b83673baf2eb03de69/packages/metrics/src/Metrics.ts#L477-L480

considerations:

• Use case: The method addMetric of Metrics which is defined in the interface MetricsInterface is extended with the new parameter resolution which is optional. The class Metrics that implements the MetricsInterface does not provide visual feedback about the new optional parameter resolution, hence optional.

However the IDE could have provided visual feedback if in the Metrics Tests, the metrics object is of type MetricsInterface. Hence the abstraction of MetricInterface is expected when extending the unit tests because of the introduction of new functionality that should be implemented in the concrete implementation of Metrics.

desribe(...) {
  test(...) {
    const metrics = new Metrics() as MetricsInterface ; 
    metrics.addMetric('test_name', MetricUnits.Seconds, MetricResolution.High);
  }
}

considerations:

• a guide or good example for the contributor to write comments in code is needed. The example may be a reference package , e.g Logger or Tracer which can improve the Metrics package for comments in code.
• reference example can indicate when to write examples , when to comment types , when to provide comment for optional parameters and default value.
• a balanced solution for rendering comments in code for using in IDE and type doc is needed.

considerations :
helper functions that can be called in one line for prepare of unit tests with meaningful name, like createMetric(), addHighResolutionMetric() that can be reused in tests and contributors can read the encapsulated code . That can improve readability and learning .

Acknowledgment

• This request meets Lambda Powertools Tenets
• Should this be considered in other Lambda Powertools languages? i.e. Python, Java, and .NET

Future readers

Please react with 👍 and your use case to help us understand customer demand.

[am29d]

Hey Niko, thanks for raising this issue 👍.

I assume this is a collection of observations you gained when working on metric resolution PR recently, right? I will dive a bit deeper into the area and will follow up with specifics to untangle all your points.

[niko-achilles]

Hey Alexander , yes it is from the contribution perspective , in the role of contributor experience .
Andreas gave me feedback in chat that 1, 3 and 4 would be better to explain more so that the issue reader can understand the why .

And for 6 there is an existing issue for testing in backlog : #163 .
It is about strategy and the point 6 of this issue can also be considered as part of strategy .

let me know how you would like handle the observations and any further information needed .

[niko-achilles]

Alexander i have updated the description of 1,2,3,4,5,6 regarding the observations from my metric resolution PR.
Additional to 6 , also Points 3,4 are related to tests.

So 3,4,6 are test observations that provided also as input information to the issue that has focus on testing Nr. 163.

and 1,2 relative to types and 5 relative to code comments.