aws-observability / cdk-aws-observability-accelerator

CDK AWS Observability Accelerator
https://aws-observability.github.io/cdk-aws-observability-accelerator/
MIT No Attribution
144 stars 37 forks source link

Changes to pattern folder name to remove construct from it and referencing correct path in .ts files under bin and respective changes in README.md file #91

Closed naveen-bathula closed 1 year ago

naveen-bathula commented 1 year ago

Changes to pattern folder name to remove construct from it and referencing correct path in .ts files under bin and respective changes in README.md file

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.

naveen-bathula commented 1 year ago

@naveen-bathula Thankyou for the PR. Could you please rename construct in all lib ts files with pattern than dropping it. Also all class names should have construct replaced with pattern. Also please review two patterns and provide testing screenshots. I prefer AWS Native new cluster and Open source existing cluster make sure to update docs if they are impacted for any names from make command outputs.

@elamaran11 All the requested changes have been made and two suggested patterns were tested and screenshots attached.

CloudWatch1 CloudWatch2 ![CloudWatch3 Grafana2

Grafana1 ](https://github.com/aws-observability/cdk-aws-observability-accelerator/assets/32726946/6012ff6f-a2c0-451f-916b-77864626c94c)

elamaran11 commented 1 year ago

@shapirov103 Can you quickly check if this is looking good for issue we had.

shapirov103 commented 1 year ago

@naveen-bathula please address the github action error https://github.com/aws-observability/cdk-aws-observability-accelerator/actions/runs/5907300979/job/16085531340?pr=91

rm -rf dist && node node_modules/.bin/tsc --skipLibCheck
Error: lib/existing-eks-opensource-observability-pattern/index.ts(6[5](https://github.com/aws-observability/cdk-aws-observability-accelerator/actions/runs/5907300979/job/16085531340?pr=91#step:7:6),17): error TS2345: Argument of type '{ bootstrapRepo: { repoUrl: string; name: string; targetRevision: string; path: string; }; fluxTargetNamespace: string; bootstrapValues: { AMG_AWS_REGION: string; AMP_ENDPOINT_URL: string; AMG_ENDPOINT_URL: string | undefined; ... 5 more ...; GRAFANA_WORKLOADS_DASH_URL: string; }; }' is not assignable to parameter of type 'FluxCDAddOnProps'.
  Object literal may only specify known properties, and 'bootstrapRepo' does not exist in type 'FluxCDAddOnProps'.
Error: lib/single-new-eks-opensource-observability-pattern/graviton-index.ts(49,17): error TS2345: Argument of type '{ bootstrapRepo: { repoUrl: string; name: string; targetRevision: string; path: string; }; fluxTargetNamespace: string; bootstrapValues: { AMG_AWS_REGION: string; AMP_ENDPOINT_URL: string; AMG_ENDPOINT_URL: string | undefined; ... 5 more ...; GRAFANA_WORKLOADS_DASH_URL: string; }; }' is not assignable to parameter of type 'FluxCDAddOnProps'.
  Object literal may only specify known properties, and 'bootstrapRepo' does not exist in type 'FluxCDAddOnProps'.
Error: lib/single-new-eks-opensource-observability-pattern/index.ts(51,1[7](https://github.com/aws-observability/cdk-aws-observability-accelerator/actions/runs/5907300979/job/16085531340?pr=91#step:7:8)): error TS2345: Argument of type '{ bootstrapRepo: { repoUrl: string; name: string; targetRevision: string; path: string; }; fluxTargetNamespace: string; bootstrapValues: { AMG_AWS_REGION: string; AMP_ENDPOINT_URL: string; AMG_ENDPOINT_URL: string | undefined; ... 5 more ...; GRAFANA_WORKLOADS_DASH_URL: string; }; }' is not assignable to parameter of type 'FluxCDAddOnProps'.
  Object literal may only specify known properties, and 'bootstrapRepo' does not exist in type 'FluxCDAddOnProps'.
make: *** [build] Error 2
Error: Process completed with exit code 2.
elamaran11 commented 1 year ago

LGTM

@naveen-bathula I suppose you captured all the doc changes as well. It sounds a bit odd that there ~10 patterns and only two changes in the docs.

This is because we are not changing the name of the pattern only the file names and class names. @naveen-bathula Please double confirm on Mikhail's feedback.

elamaran11 commented 1 year ago

@naveen-bathula please address the github action error https://github.com/aws-observability/cdk-aws-observability-accelerator/actions/runs/5907300979/job/16085531340?pr=91

rm -rf dist && node node_modules/.bin/tsc --skipLibCheck
Error: lib/existing-eks-opensource-observability-pattern/index.ts(6[5](https://github.com/aws-observability/cdk-aws-observability-accelerator/actions/runs/5907300979/job/16085531340?pr=91#step:7:6),17): error TS2345: Argument of type '{ bootstrapRepo: { repoUrl: string; name: string; targetRevision: string; path: string; }; fluxTargetNamespace: string; bootstrapValues: { AMG_AWS_REGION: string; AMP_ENDPOINT_URL: string; AMG_ENDPOINT_URL: string | undefined; ... 5 more ...; GRAFANA_WORKLOADS_DASH_URL: string; }; }' is not assignable to parameter of type 'FluxCDAddOnProps'.
  Object literal may only specify known properties, and 'bootstrapRepo' does not exist in type 'FluxCDAddOnProps'.
Error: lib/single-new-eks-opensource-observability-pattern/graviton-index.ts(49,17): error TS2345: Argument of type '{ bootstrapRepo: { repoUrl: string; name: string; targetRevision: string; path: string; }; fluxTargetNamespace: string; bootstrapValues: { AMG_AWS_REGION: string; AMP_ENDPOINT_URL: string; AMG_ENDPOINT_URL: string | undefined; ... 5 more ...; GRAFANA_WORKLOADS_DASH_URL: string; }; }' is not assignable to parameter of type 'FluxCDAddOnProps'.
  Object literal may only specify known properties, and 'bootstrapRepo' does not exist in type 'FluxCDAddOnProps'.
Error: lib/single-new-eks-opensource-observability-pattern/index.ts(51,1[7](https://github.com/aws-observability/cdk-aws-observability-accelerator/actions/runs/5907300979/job/16085531340?pr=91#step:7:8)): error TS2345: Argument of type '{ bootstrapRepo: { repoUrl: string; name: string; targetRevision: string; path: string; }; fluxTargetNamespace: string; bootstrapValues: { AMG_AWS_REGION: string; AMP_ENDPOINT_URL: string; AMG_ENDPOINT_URL: string | undefined; ... 5 more ...; GRAFANA_WORKLOADS_DASH_URL: string; }; }' is not assignable to parameter of type 'FluxCDAddOnProps'.
  Object literal may only specify known properties, and 'bootstrapRepo' does not exist in type 'FluxCDAddOnProps'.
make: *** [build] Error 2
Error: Process completed with exit code 2.

@naveen-bathula Lets this fix since you are already having an active PR. Check this doc for an example to fix it. This is just parameter fix and you can turn it on quickly and just do a quick test. This is because of blueprints flux change breaking the accelerator. Please fix this asap.

elamaran11 commented 1 year ago

@shapirov103 Appreciate if you can take a quick look.

naveen-bathula commented 1 year ago

LGTM. Please let me know once your e2e is working @naveen-bathula We can merge it.

@elamaran11

I have tested single-new-eks-opensource-observability and existing-eks-opensource-observability patterns, attached screenshots

Grafana3

Grafana4

elamaran11 commented 1 year ago

Nice work @naveen-bathula, Merged.