Add QueryParameters option to AWS CloudMap-based peer discovery

[engechas]

Description

This PR adds awsCloudMapQueryParameters as an optional setting when using AWS CloudMap for peer discovery. The implementation accepts a Map<String, String> which is passed directly to the queryParameters field of the DiscoverInstancesRequest.

Documentation on this field: https://sdk.amazonaws.com/java/api/latest/software/amazon/awssdk/services/servicediscovery/model/DiscoverInstancesRequest.Builder.html#queryParameters-java.util.Map-

Testing

AWS CloudMap service contents:

aws servicediscovery --region eu-west-1 discover-instances --namespace serv-disc --service serv-disc
{
    "Instances": [
        {
            "InstanceId": "a2ae948ed1244897bf8dfa404fc9593c",
            "NamespaceName": "serv-disc",
            "ServiceName": "serv-disc",
            "HealthStatus": "HEALTHY",
            "Attributes": {
                "AWS_INSTANCE_IPV4": "<redacted>",
                "REGION": "eu-west-1"
            }
        },
        {
            "InstanceId": "30e368d910bf421e98a246d7275987ba",
            "NamespaceName": "serv-disc",
            "ServiceName": "serv-disc",
            "HealthStatus": "HEALTHY",
            "Attributes": {
                "AWS_INSTANCE_IPV4": "<redacted>",
                "REGION": "eu-west-1",
                "test": "filter-params"
            }
        },
        {
            "InstanceId": "c18c00a21852483ebefbb020510bb910",
            "NamespaceName": "serv-disc",
            "ServiceName": "serv-disc",
            "HealthStatus": "HEALTHY",
            "Attributes": {
                "AWS_INSTANCE_IPV4": "<redacted>",
                "REGION": "eu-west-1"
            }
        }
    ]
}

Testing no filters

pipelines.yaml:

simple-sample-pipeline:
  workers: 2
  delay: "5000"
  source:
    random:
  processor:
    - peer_forwarder:
        discovery_mode: aws_cloud_map
        ssl: false
        awsCloudMapNamespaceName: serv-disc
        awsCloudMapServiceName: serv-disc
        awsRegion: eu-west-1
  sink:
    - stdout:

Logs:

2022-07-01T22:41:13,504 [sdk-async-response-0-0] INFO  com.amazon.dataprepper.plugins.prepper.peerforwarder.discovery.AwsCloudMapPeerListProvider$AwsCloudMapDynamicEndpointGroup - Discovered 3 instances.

Testing filter has match:

pipelines.yaml:

simple-sample-pipeline:
  workers: 2
  delay: "5000"
  source:
    random:
  processor:
    - peer_forwarder:
        discovery_mode: aws_cloud_map
        ssl: false
        awsCloudMapNamespaceName: serv-disc
        awsCloudMapServiceName: serv-disc
        awsRegion: eu-west-1
        awsCloudMapQueryParameters:
          test: filter-params
  sink:
    - stdout:

Logs:

2022-07-01T22:58:23,312 [sdk-async-response-0-0] INFO  com.amazon.dataprepper.plugins.prepper.peerforwarder.discovery.AwsCloudMapPeerListProvider$AwsCloudMapDynamicEndpointGroup - Discovered 1 instances.

Testing filter has no matches

pipelines.yaml:

simple-sample-pipeline:
  workers: 2
  delay: "5000"
  source:
    random:
  processor:
    - peer_forwarder:
        discovery_mode: aws_cloud_map
        ssl: false
        awsCloudMapNamespaceName: serv-disc
        awsCloudMapServiceName: serv-disc
        awsRegion: eu-west-1
        awsCloudMapQueryParameters:
          test: no-match
  sink:
    - stdout:

Logs:

2022-07-01T22:59:47,138 [sdk-async-response-0-0] INFO  com.amazon.dataprepper.plugins.prepper.peerforwarder.discovery.AwsCloudMapPeerListProvider$AwsCloudMapDynamicEndpointGroup - Discovered 0 instances.
2022-07-01T22:59:47,141 [sdk-async-response-0-0] INFO  com.amazon.dataprepper.plugins.prepper.peerforwarder.HashRing - Building hash ring with endpoints: []

Testing blank entry

pipelines.yaml:

simple-sample-pipeline:
  workers: 2
  delay: "5000"
  source:
    random:
  processor:
    - peer_forwarder:
        discovery_mode: aws_cloud_map
        ssl: false
        awsCloudMapNamespaceName: serv-disc
        awsCloudMapServiceName: serv-disc
        awsRegion: eu-west-1
        awsCloudMapQueryParameters:
  sink:
    - stdout:

Logs:

2022-07-01T23:02:33,129 [sdk-async-response-0-0] INFO  com.amazon.dataprepper.plugins.prepper.peerforwarder.discovery.AwsCloudMapPeerListProvider$AwsCloudMapDynamicEndpointGroup - Discovered 3 instances.

Check List

• New functionality includes testing.
• New functionality has been documented.
  • [N/A] New functionality has javadoc added
• Commits are signed with a real name per the DCO

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.

[codecov-commenter]

Codecov Report

Merging #1560 (af6aeda) into main (5febd6e) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               main    #1560   +/-   ##
=========================================
  Coverage     94.18%   94.18%           
  Complexity     1181     1181           
=========================================
  Files           165      165           
  Lines          3386     3386           
  Branches        277      277           
=========================================
  Hits           3189     3189           
  Misses          141      141           
  Partials         56       56           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5febd6e...af6aeda. Read the comment docs.

[cmanning09]

Out of scope for PR: We should eliminate null checks by enforcing a default value in the PluginSettings or allow calling classes to specify the default. Raising: #1563

[dlvenable]

Thanks! This is the bit of code that I was hoping to test.

The only missing part is to verify that a subsequent call to the AWS SDK has an empty map.

[engechas]

The ServiceDiscoveryAsyncClient is created as a local variable in the createPeerListProvider so there's not a good way to mock it in this flow without introducing PowerMock as a new dependency or a decent chunk of refactoring.

I will drop the private scope from the getOptionalSettingMap to test the various scenarios while keeping this PR minimal.

[dlvenable]

OK. I'm fine with keeping this as it is for now.