Remove InstrumentationLibrary dimension in CloudWatch EMF Logs if it is undefined #1256
Conversation
…is not defined in metrics
| fieldsPairs[OtlibDimensionKey] = OTLib | ||
| dimensionArray = append(dimensionArray, append(dimensionSlice, OtlibDimensionKey)) | ||
| // add OTel instrumentation lib name as an additional dimension if it is defined | ||
| if instrumentationLibName != noInstrumentationLibraryName { |
There was a problem hiding this comment.
What happens if we set "unknown", "unset", or similar instead of filtering? Doesn't it allow the metrics to be more symmetric (e.g., when analyzing metrics based on this dimension, all the data will be present instead of having a gap)?
There was a problem hiding this comment.
The assumption is that the metrics generated by OTel SDKs should not have NonNull InstrumentationLibrary and this dimension should never be filtered out on these metrics. The complaints are from AWSECSMetadataReceiver and StatsDReceiver users saying why they are having OTelLib dimension in their metrics but they're not using any of instrumentation lib to generate metrics. Thanks
|
Hi @anuraaga , @shaochengwang . Please help to review |
| if len(originalDimensionSlice) > 0 && len(dimensionZero) > 0 { | ||
| rollupDimensionArray = append(rollupDimensionArray, dimensionZero) | ||
| } |
There was a problem hiding this comment.
What will happen if one metric has several dimensions but do not have instrumentationLibName defined? Can it have Zero dimension rollup?
For example, Metric1 has Dimension A and B, but do not have instrumentationLibName. If customer enables ZeroDimensionRollup, will he still get the Metric1 without any dimension? It looks like you didn't append [] into rollupDimensionArray if instrumentationLibName == noInstrumentationLibraryName. Or is this something we expect?
There was a problem hiding this comment.
Yes, we should generate zero dimension metric for this case. Made the update and added test cases to cover these combinations. Thanks.
…dded test cases to cover all the combinations
Description:
In the current AWSEMFExporter, InstrumentationLibrary name is a fixed dimension when converting OTel metrics to CloudWatch EMF(structure logs) metrics. From the testing, there are some cases OTel metrics can be generated without InstrumentationLibrary attached. Eg,
AWSECSMetadataReceivergenerates ECS container Infra metrics without InstrumentationLibrary. In this case, we'd like to not add InstrumentationLibrary name as the dimension since it will generated the dimension value withUndefined.Link to tracking Issue:
N/A
Testing:
unit test