Migration Analytics Services to CDK #417

mikaylathompson · 2023-11-13T23:59:04Z

Description

This PR brings the otel collector & opensearch domain for analytics established in #376 to our CDK solution for AWS. In a default deployment, it deploys the below components and adds the --otelCollectorEndpoint to the capture proxy and replayer so that they initialize the OpenTelemetry adapters and begin sending data.

Major Components:

Migration Analytics Cluster & Dashboard (OpenSearch Service Domain)
Open Telemetry Collector (ECS Fargate box running a custom build of OpenTelemetry [see here for details])
Bastion Host that allows a user to access their dashboard via port forwarding set up via a Session Manager connection (similar to how it's used in our accessContainer.sh script)

Missing documentation:

how to access the bastion host

Issues Resolved

MIGRATIONS-1415

Testing

This has been subject to a lot of manual testing, but the CDK itself is only covered by one fairly minimal test at this point.

Check List

New functionality includes testing
- All tests pass, including unit test, integration test and doctest
New functionality has been documented
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Mikayla Thompson <[email protected]>

codecov · 2023-11-14T00:10:30Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9eed2d7) 63.16% compared to head (94cbe90) 63.16%.
Report is 5 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##               main     #417   +/-   ##
=========================================
  Coverage     63.16%   63.16%           
  Complexity      873      873           
=========================================
  Files           100      100           
  Lines          4341     4341           
  Branches        410      410           
=========================================
  Hits           2742     2742           
  Misses         1328     1328           
  Partials        271      271

Flag	Coverage Δ
unittests	`63.16% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

lewijacn

I think you already mentioned this in your update that documentation hadn't been added, but would love to see some documentation on steps a user needs to access the analytics dashboard

lewijacn · 2023-11-14T01:32:11Z

deployment/cdk/opensearch-service-migration/lib/service-stacks/migration-console-stack.ts

        ]
+        if (props.migrationAnalyticsEnabled) {
+            securityGroups.push(SecurityGroup.fromSecurityGroupId(this, "migrationAnalyticsSGId", StringParameter.valueForStringParameter(this, `/migration/${props.stage}/${props.defaultDeployId}/analyticsDomainSGId`)))


Nothing to do about this right now, but we should rethink our security group pattern later. I think there may be a limit of 5 security groups per ECS service, and I think in our effort to try and limit access by valid use case we have reached that limit and may need to be more general

Ah, that's good to know.

deployment/cdk/opensearch-service-migration/lib/stack-composer.ts

deployment/cdk/opensearch-service-migration/lib/service-stacks/migration-analytics-stack.ts

lewijacn · 2023-11-14T03:11:51Z

deployment/cdk/opensearch-service-migration/lib/stack-composer.ts

+                description: "This stack prepares the Migration Analytics OS Domain",
+                domainAccessSecurityGroupParameter: "analyticsDomainSGId",
+                endpointParameterName: "analyticsDomainEndpoint",
+                version: this.getEngineVersion(analyticsDomainEngineVersion ?? engineVersion),  // If no analytics version is specified, use the same as the target cluster


nit: I'd like to use the opensearch latest by default, but understand there isn't a built in way to get this

deployment/cdk/opensearch-service-migration/lib/service-stacks/migration-analytics-stack.ts

lewijacn · 2023-11-14T03:56:08Z

deployment/cdk/opensearch-service-migration/lib/stack-composer.ts

+                vpcSubnetIds: vpcSubnetIds,
+                vpcSecurityGroupIds: vpcSecurityGroupIds,
+                availabilityZoneCount: availabilityZoneCount,
+                dataNodeInstanceType: analyticsDomainDataNodeType,
+                dataNodes: analyticsDomainDataNodeCount ?? availabilityZoneCount,  // There's probably a better way to do this, but the node count must be >= the zone count, and possibly must be the same even/odd as zone count


TBD Comment, need to think about this a bit more

Signed-off-by: Mikayla Thompson <[email protected]>

lewijacn

Things look good from what I can see. Any issue to run npm test? I was unable to get these successful but it seems like an issue with my local

lewijacn · 2023-11-15T01:35:02Z

deployment/cdk/opensearch-service-migration/lib/stack-composer.ts

+            if (migrationAnalyticsStack) {
+                migrationConsoleStack.addDependency(migrationAnalyticsStack)


We are missing bracket here

Ah, yikes, good catch.

lewijacn

Marking as approved but we should add the missing bracket in my last review before merging to prevent messing up console and tests

Signed-off-by: Mikayla Thompson <[email protected]>

mikaylathompson · 2023-11-15T05:07:30Z

And yes, all tests pass:

Test Suites: 7 passed, 7 total
Tests:       41 passed, 41 total
Snapshots:   0 total
Time:        182.867 s
Ran all test suites.

mikaylathompson added 9 commits November 13, 2023 16:26

Initial commit for creating otel container in cdk

5bc4b80

Signed-off-by: Mikayla Thompson <[email protected]>

Debugging otelcol build

e678a1b

Signed-off-by: Mikayla Thompson <[email protected]>

First pass at adding analytics cluster

2228b16

Signed-off-by: Mikayla Thompson <[email protected]>

Handle SG parameter & stack dependencies

3adddb8

Signed-off-by: Mikayla Thompson <[email protected]>

Fix the two-stacks/same-id issue, update SG usage

25c5655

Signed-off-by: Mikayla Thompson <[email protected]>

Fix issue with inherited stackName & add bastion host (untested)

d77d41b

Signed-off-by: Mikayla Thompson <[email protected]>

Refactor how domain is created, add service connect, update dockerfile

06cbab3

Signed-off-by: Mikayla Thompson <[email protected]>

Final fixes to get data moving

0b7c87d

Signed-off-by: Mikayla Thompson <[email protected]>

Cleanup the code and add a test

54c262d

Signed-off-by: Mikayla Thompson <[email protected]>

lewijacn reviewed Nov 14, 2023

View reviewed changes

Address review comments

09d33a8

Signed-off-by: Mikayla Thompson <[email protected]>

mikaylathompson marked this pull request as ready for review November 14, 2023 19:15

mikaylathompson requested review from chelma, gregschohn, kartg, okhasawn and sumobrian as code owners November 14, 2023 19:15

mikaylathompson added 3 commits November 14, 2023 12:56

Add script + doc note to access cluster via bastion host

fce7552

Signed-off-by: Mikayla Thompson <[email protected]>

Merge branch 'main' into analytics-domain-service-in-cdk

603fb09

Signed-off-by: Mikayla Thompson <[email protected]>

Make bastion host optional (defaults to disabled)

b0a8120

Signed-off-by: Mikayla Thompson <[email protected]>

mikaylathompson requested a review from lewijacn November 15, 2023 00:24

lewijacn approved these changes Nov 15, 2023

View reviewed changes

lewijacn requested changes Nov 15, 2023

View reviewed changes

lewijacn approved these changes Nov 15, 2023

View reviewed changes

Fix missing bracket

94cbe90

Signed-off-by: Mikayla Thompson <[email protected]>

mikaylathompson merged commit 73b076f into opensearch-project:main Nov 15, 2023
7 checks passed

mikaylathompson deleted the analytics-domain-service-in-cdk branch November 15, 2023 05:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migration Analytics Services to CDK #417

Migration Analytics Services to CDK #417

mikaylathompson commented Nov 13, 2023 •

edited

Loading

codecov bot commented Nov 14, 2023 •

edited

Loading

lewijacn left a comment •

edited

Loading

lewijacn Nov 14, 2023

mikaylathompson Nov 14, 2023

lewijacn Nov 14, 2023

lewijacn Nov 14, 2023

lewijacn left a comment

lewijacn Nov 15, 2023

mikaylathompson Nov 15, 2023

lewijacn left a comment

mikaylathompson commented Nov 15, 2023

		if (migrationAnalyticsStack) {
		migrationConsoleStack.addDependency(migrationAnalyticsStack)

Migration Analytics Services to CDK #417

Migration Analytics Services to CDK #417

Conversation

mikaylathompson commented Nov 13, 2023 • edited Loading

Description

Issues Resolved

Testing

Check List

codecov bot commented Nov 14, 2023 • edited Loading

Codecov Report

lewijacn left a comment • edited Loading

Choose a reason for hiding this comment

lewijacn Nov 14, 2023

Choose a reason for hiding this comment

mikaylathompson Nov 14, 2023

Choose a reason for hiding this comment

lewijacn Nov 14, 2023

Choose a reason for hiding this comment

lewijacn Nov 14, 2023

Choose a reason for hiding this comment

lewijacn left a comment

Choose a reason for hiding this comment

lewijacn Nov 15, 2023

Choose a reason for hiding this comment

mikaylathompson Nov 15, 2023

Choose a reason for hiding this comment

lewijacn left a comment

Choose a reason for hiding this comment

mikaylathompson commented Nov 15, 2023

mikaylathompson commented Nov 13, 2023 •

edited

Loading

codecov bot commented Nov 14, 2023 •

edited

Loading

lewijacn left a comment •

edited

Loading