Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support AWS Embedded Metric Format Version 0 #20314

Merged
merged 3 commits into from
Apr 25, 2023
Merged

Add support AWS Embedded Metric Format Version 0 #20314

merged 3 commits into from
Apr 25, 2023

Conversation

khanhntd
Copy link
Contributor

@khanhntd khanhntd commented Mar 25, 2023 •
edited
Loading

Description:
Currently, awsemfexporter will group all the metrics, translate to fields and setting the fields with exported metrics as EMF version 1 -

"_aws": {
					"CloudWatchMetrics": [
					{
						"Namespace": "ECS",
						"Dimensions": [ ["ClusterName"] ],
						"Metrics": [{"Name": "memcached_commands_total"}]
					}
					],
					"Timestamp": 1668387032641
			  	}

Therefore, adding support for EMF version v0 and a flag for customers to make a difference between Version 0 and Version 1 (e.g EMF v0)

				{
					"Version": 0,
					"CloudWatchMetrics": [
					{
						"Namespace": "ECS",
						"Dimensions": [ ["ClusterName"] ],
						"Metrics": [{"Name": "memcached_commands_total"}]
					}
					],
					"Timestamp": 1668387032641
			  	}

This can be validated by send metrics with awsemfexporter

image

Link to tracking Issue:
N/A

Testing:
There are two tests I have done:

  • By running go test with awsemfexporter, the result is
khanhntd@88665a0eaf54 awsemfexporter % go test ./...
ok      github.com/open-telemetry/opentelemetry-collector-contrib/exporter/awsemfexporter       2.409s

@github-actions github-actions bot added the exporter/awsemf awsemf exporter label Mar 25, 2023
@runforesight
Copy link

runforesight bot commented Mar 25, 2023 •
edited
Loading

Foresight Summary

    
Major Impacts

build-and-test duration(34 minutes 36 seconds) has decreased 11 minutes 45 seconds compared to main branch avg(46 minutes 21 seconds).
View More Details

⭕  build-and-test-windows workflow has finished in 5 seconds (31 minutes 35 seconds less than main branch avg.) and finished at 5th Apr, 2023.

Job Failed Steps Tests
windows-unittest-matrix -     🔗  N/A See Details
windows-unittest -     🔗  N/A See Details

✅  telemetrygen workflow has finished in 1 minute 1 second and finished at 5th Apr, 2023.

Job Failed Steps Tests
build-dev -     🔗  N/A See Details
publish-latest -     🔗  N/A See Details
publish-stable -     🔗  N/A See Details

✅  check-links workflow has finished in 1 minute 34 seconds (⚠️ 36 seconds more than main branch avg.) and finished at 5th Apr, 2023.

Job Failed Steps Tests
changed files -     🔗  N/A See Details
check-links -     🔗  N/A See Details

✅  build-and-test workflow has finished in 34 minutes 36 seconds (11 minutes 45 seconds less than main branch avg.) and finished at 5th Apr, 2023.

Job Failed Steps Tests
unittest-matrix (1.20, connector) -     🔗  ✅ 126  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.19, connector) -     🔗  ✅ 126  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.19, internal) -     🔗  ✅ 583  ❌ 0  ⏭ 0    🔗 See Details
correctness-metrics -     🔗  ✅ 2  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.20, processor) -     🔗  ✅ 1557  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.20, internal) -     🔗  ✅ 583  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.19, extension) -     🔗  ✅ 544  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.20, extension) -     🔗  ✅ 544  ❌ 0  ⏭ 0    🔗 See Details
correctness-traces -     🔗  ✅ 17  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.19, processor) -     🔗  ✅ 1557  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.20, receiver-0) -     🔗  ✅ 2616  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.19, receiver-0) -     🔗  ✅ 2616  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.20, exporter) -     🔗  ✅ 2512  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.19, receiver-1) -     🔗  ✅ 1960  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.20, receiver-1) -     🔗  ✅ 1960  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.19, exporter) -     🔗  ✅ 2512  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.19, other) -     🔗  ✅ 4761  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.20, other) -     🔗  ✅ 4761  ❌ 0  ⏭ 0    🔗 See Details
integration-tests -     🔗  ✅ 56  ❌ 0  ⏭ 0    🔗 See Details
setup-environment -     🔗  N/A See Details
lint-matrix (receiver-0) -     🔗  N/A See Details
lint-matrix (receiver-1) -     🔗  N/A See Details
lint-matrix (processor) -     🔗  N/A See Details
lint-matrix (exporter) -     🔗  N/A See Details
lint-matrix (extension) -     🔗  N/A See Details
lint-matrix (connector) -     🔗  N/A See Details
lint-matrix (internal) -     🔗  N/A See Details
lint-matrix (other) -     🔗  N/A See Details
checks -     🔗  N/A See Details
check-collector-module-version -     🔗  N/A See Details
check-codeowners -     🔗  N/A See Details
build-examples -     🔗  N/A See Details
lint -     🔗  N/A See Details
unittest (1.20) -     🔗  N/A See Details
unittest (1.19) -     🔗  N/A See Details
cross-compile (darwin, amd64) -     🔗  N/A See Details
cross-compile (darwin, arm64) -     🔗  N/A See Details
cross-compile (linux, 386) -     🔗  N/A See Details
cross-compile (linux, amd64) -     🔗  N/A See Details
cross-compile (linux, arm) -     🔗  N/A See Details
cross-compile (linux, arm64) -     🔗  N/A See Details
cross-compile (windows, 386) -     🔗  N/A See Details
cross-compile (linux, ppc64le) -     🔗  N/A See Details
cross-compile (windows, amd64) -     🔗  N/A See Details
build-package (deb) -     🔗  N/A See Details
build-package (rpm) -     🔗  N/A See Details
windows-msi -     🔗  N/A See Details
publish-check -     🔗  N/A See Details
publish-dev -     🔗  N/A See Details
publish-stable -     🔗  N/A See Details
rotate-milestone -     🔗  N/A See Details

✅  changelog workflow has finished in 2 minutes 31 seconds and finished at 5th Apr, 2023.

Job Failed Steps Tests
changelog -     🔗  N/A See Details

✅  prometheus-compliance-tests workflow has finished in 3 minutes 8 seconds (3 minutes 11 seconds less than main branch avg.) and finished at 5th Apr, 2023.

Job Failed Steps Tests
prometheus-compliance-tests -     🔗  ✅ 21  ❌ 0  ⏭ 0    🔗 See Details

✅  load-tests workflow has finished in 7 minutes 7 seconds (3 minutes 11 seconds less than main branch avg.) and finished at 5th Apr, 2023.

Job Failed Steps Tests
loadtest (TestTraceAttributesProcessor) -     🔗  ✅ 3  ❌ 0  ⏭ 0    🔗 See Details
loadtest (TestIdleMode) -     🔗  ✅ 1  ❌ 0  ⏭ 0    🔗 See Details
loadtest (TestMetric10kDPS|TestMetricsFromFile) -     🔗  ✅ 6  ❌ 0  ⏭ 0    🔗 See Details
loadtest (TestTraceNoBackend10kSPS|TestTrace1kSPSWithAttrs) -     🔗  ✅ 8  ❌ 0  ⏭ 0    🔗 See Details
loadtest (TestMetricResourceProcessor|TestTrace10kSPS) -     🔗  ✅ 12  ❌ 0  ⏭ 0    🔗 See Details
loadtest (TestTraceBallast1kSPSWithAttrs|TestTraceBallast1kSPSAddAttrs) -     🔗  ✅ 10  ❌ 0  ⏭ 0    🔗 See Details
loadtest (TestBallastMemory|TestLog10kDPS) -     🔗  ✅ 18  ❌ 0  ⏭ 0    🔗 See Details
setup-environment -     🔗  N/A See Details

✅  e2e-tests workflow has finished in 12 minutes 13 seconds (1 minute 51 seconds less than main branch avg.) and finished at 5th Apr, 2023.

Job Failed Steps Tests
kubernetes-test (v1.25.3) -     🔗  N/A See Details
kubernetes-test (v1.26.0) -     🔗  N/A See Details
kubernetes-test (v1.24.7) -     🔗  N/A See Details
kubernetes-test (v1.23.13) -     🔗  N/A See Details

🔎 See details on Foresight

*You can configure Foresight comments in your organization settings page.

@khanhntd khanhntd force-pushed the emf branch 2 times, most recently from 02cd627 to a3d4b05 Compare March 31, 2023 12:24
@khanhntd khanhntd force-pushed the emf branch 3 times, most recently from 1f37629 to a3836d9 Compare March 31, 2023 13:30
khanhntd
khanhntd commented Mar 31, 2023
View reviewed changes
exporter/awsemfexporter/metric_declaration.go
lm.compiledRegex = regexp.MustCompile(lm.Regex)
return

lm.compiledRegex, err = regexp.Compile(lm.Regex)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MustCompile will panic if the regexp is not compilable. Therefore, using Compile which are the same with MustCompile but return readable error instead of panic

exporter/awsemfexporter/factory.go Outdated
@@ -47,18 +49,32 @@ func createDefaultConfig() component.Config {
LogStreamName: "",
Namespace: "",
DimensionRollupOption: "ZeroAndSingleDimensionRollup",
EnableEMFVersion1: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default EMF version is v1 and the public document mentions V1 more than V0. Therefore, enable EMF version with default is true

exporter/awsemfexporter/config.go
@@ -102,6 +108,8 @@ type MetricDescriptor struct {
Overwrite bool `mapstructure:"overwrite"`
}

var _ component.Config = (*Config)(nil)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will validate if the Config declared by awsemfexporter Config has all the declared function by the interface

@khanhntd khanhntd changed the title Support AWS EMF Version 1 Add support AWS Embedded Metric Format Version 0 Mar 31, 2023
@khanhntd khanhntd force-pushed the emf branch 3 times, most recently from 1f82242 to 78d20c5 Compare March 31, 2023 13:45
@khanhntd khanhntd marked this pull request as ready for review March 31, 2023 13:47
@khanhntd khanhntd requested a review from a team March 31, 2023 13:47
@khanhntd khanhntd requested a review from Aneurysm9 as a code owner March 31, 2023 13:47
@khanhntd khanhntd force-pushed the emf branch 5 times, most recently from 0e14548 to 9a6d4d5 Compare March 31, 2023 15:51
@khanhntd khanhntd mentioned this pull request Apr 3, 2023
Merged
@khanhntd khanhntd force-pushed the emf branch 6 times, most recently from 5242664 to d908ab5 Compare April 4, 2023 20:56
@khanhntd khanhntd force-pushed the emf branch 4 times, most recently from 30fc312 to a14f469 Compare April 4, 2023 21:15
@khanhntd khanhntd mentioned this pull request Apr 4, 2023
Closed
@khanhntd
Copy link
Contributor Author

Hi Alex (@codeboten), would you able to help me reviewing the PR since Sean is out of the office. Thanks so much for the help though.

MovieStoreGuy
MovieStoreGuy approved these changes Apr 19, 2023
View reviewed changes
Copy link
Contributor

@MovieStoreGuy MovieStoreGuy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes make sense to me, but I will leave to @Aneurysm9 as codeowner to ensure.

@MovieStoreGuy MovieStoreGuy added the ready to merge Code review completed; ready to merge by maintainers label Apr 19, 2023
@codeboten codeboten merged commit 132b116 into open-telemetry:main Apr 25, 2023
@github-actions github-actions bot added this to the next release milestone Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MovieStoreGuy MovieStoreGuy MovieStoreGuy approved these changes

@Aneurysm9 Aneurysm9 Aneurysm9 approved these changes

@codeboten codeboten codeboten approved these changes

Assignees

@MovieStoreGuy MovieStoreGuy

Labels
exporter/awsemf awsemf exporter ready to merge Code review completed; ready to merge by maintainers
Projects
None yet
Milestone
v0.76.3
Development

Successfully merging this pull request may close these issues.
4 participants
@khanhntd @codeboten @Aneurysm9 @MovieStoreGuy