Configurations of OpenTelemetryCollector and AMP Rules for AmpAddOn #801
Conversation
freschri
requested review from
aws-ia-ci,
parkand1,
shapirov103,
youngjeong46,
elamaran11 and
a team
as code owners
There was a problem hiding this comment.
@freschri This is awesome work, appreciate the work here. Have few comments and questions. I love the approach.
| ampRules: { | ||
| ampWorkspaceArn: ampWorkspaceArn, | ||
| ruleFilePaths: [ | ||
| __dirname + '/../common/resources/amp-config/alerting-rules.yml', |
There was a problem hiding this comment.
You can add couple of statements to say these files should be located in the code repo which is using the Addon. Also please share couple of sample alerting and recording rules underneath and point to some link for references which people can use to setup.
docs/addons/amp-addon.md
Outdated
| .build(app, 'my-stack-name'); | ||
| ``` | ||
|
|
||
| Pattern # 6 : Configuring the [AWS Distro for OpenTelemetry Collector](https://aws-otel.github.io/docs/getting-started/collector). This pattern enables you to configure the Collector by providing a manifest describing an OpenTelemetryCollector resource: |
There was a problem hiding this comment.
Extra space after a manifest. Also can you explain a little bit more clearly about this pattern. The explanation is not clear may be focus a bit on usecase.
lib/addons/amp/index.ts
Outdated
| name?: string; | ||
| name?: string; | ||
| /** | ||
| * AMP rules. |
There was a problem hiding this comment.
Elaborate. Do we have any defaults?
test/amp.test.ts
Outdated
| expect(blueprint).toBeDefined(); | ||
| }); | ||
|
|
||
| test("Stack creation fails due to missing AdotCollectorAddOn", () => { |
There was a problem hiding this comment.
I think this test is not required because we have a dependency with ADOT Addon in AmpAddon. But if you prefer to keep it im good.
lib/addons/amp/index.ts
Outdated
| const manifest = doc.split("---").map(e => { | ||
| let object = loadYaml(e); | ||
|
|
||
| if (this.ampAddOnProps.openTelemetryCollectorManifestPath !== undefined && object.kind === "OpenTelemetryCollector"){ |
There was a problem hiding this comment.
The intention here is to overwrite the yaml doc from collector-config-amp.ytpl to openTelemetryCollectorManifestPath is passed?
There was a problem hiding this comment.
correct (or collector-config-amp-daemonset.ytpl)
|
@freschri Check for lint errors |
There was a problem hiding this comment.
@freschri Great work, very minor comments/nits.
The failure of the doc checker should be addressed because otherwise it will require us an exclusion. Do you mind adding http://console-eks.yourdomain.com/ to the .github/workflows/linkcheck.json (ingorePatterns)
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.