feat(new sink): add AWS Simple Notification Service aws_sns sink

[wochinge]

Proposed Changes

• add support for AWS Simple Notification Service (SNS sink)
• This is useful to distribute logs in a fan out fashion (as opposed to AWS SQS)
  • SNS is very similar to SQS so should be quite simple to maintain this alongside SQS.
• separated out common code from SQS and SNS

Closes: #18142

[bits-bot]

All committers have signed the CLA.

[netlify]

✅ Deploy Preview for vrl-playground canceled.

Name	Link
🔨 Latest commit	185fe20
🔍 Latest deploy log	https://app.netlify.com/sites/vrl-playground/deploys/64dea276b9c5ac000822c731

[netlify]

✅ Deploy Preview for vector-project canceled.

Name	Link
🔨 Latest commit	185fe20
🔍 Latest deploy log	https://app.netlify.com/sites/vector-project/deploys/64dea27635a82500085518fc

[neuronull]

Hey @wochinge , thanks for proposing this new component.

Before you go too much further I just wanted to call out we have a procedure to follow for adding new components: https://github.com/vectordotdev/vector/blob/master/CONTRIBUTING.md#new-sources-sinks-and-transforms

, and in general it starts with a discussion, ideally a GH issue for us to essentially qualify the new component to decide if we'd want to accept it or not.

That said we can still do all of that with the PR state of the changes.

[wochinge]

Created an issue @neuronull 👍🏻 Sorry for going the wrong way around - I thought that could maybe make the decision easier.

[neuronull]

Created an issue @neuronull 👍🏻 Sorry for going the wrong way around - I thought that could maybe make the decision easier.

Hey no worries- it happens a lot. We mostly want to steer towards the q&a first just to avoid folks writing code that might not be utilized.

Thanks for opening that issue and filling out some details there. We'll follow up soon with any thoughts/questions.

[neuronull]

Almost all of this code is re-usable with the SQS sink.

We can do something similar to aws_kinesis_* sinks (but shouldn't be as complex as that), where there is a common directory that contains the shared code, and then sub directories that contain the integration specific code.

The main (seemingly only) difference is the SNS operates on the topic, and the SQS operates on the queue. These are both strings, so the usage of them should be easy to converge on. And then there is a different Client used from the aws sdk.

Unfortunately it looks like the aws sdk does not seem to utilize a common Trait between the clients, despite them being so similar.
But, we can get around that by defining our own. Basically need a trait which has trait functions for any of the function calls to the aws sdk *Client. Then we can use Generics for example in the Service, instead of the client.publish().<...> directly in the call function, it would look more like

    pub(super) struct SnsService<C> {
        pub client: C,
        // ...
    }
     
    impl<C> Service<SendMessageEntry> for SnsService<C>
    where
        C: CommonClientTrait {

    fn call(&mut self, entry: SendMessageEntry) -> Self::Future {
     // ...
     self.client.send(entry).map_ok(|_| SendMessageResponse {
         // ...

And the SQS and SNS both have an implementation of the trait that handles the differences between them.

Doing that will allow us to eliminate all of the duplicate code that is shared between the components and will be easier to maintain.

I'll hold off on further feedback until then. LMK if my suggestion needs further clarification.

[wochinge]

Hey @neuronull, thanks for the feedback and the helpful pointers. I have never coded in Rust before but most of the stuff is familiar. Will give it a try in the next days. Another, FYI: I'll be on vacation until September from next Monday onwards. Might be the case that my colleague @ArzelaAscoIi takes over then :-)

[wochinge]

@neuronull One first question: How do you run the integration tests? It first failed to start the localstack setup and now it insists on running amqp tests 😁

I used make test-integration-aws but also variations like make test-integration SCOPE="sinks::aws-sqs" ENVIRONMENT=true

UPDATE:
Solved it by running make test-integration-aws. This generated a docker compose file in scripts/integrartion/aws which I started with DOCKER_SOCKET=/var/run/docker.sock docker compose up. Then I reran the tests and things passed :-D

[neuronull]

Running our integration tests locally can be a bit touchy sometimes. We're loosely aware of this.

Some pointers that could help for future reference: you can utilize the underlying vdev commands to have more control over the testing.

When in doubt and you're seeing odd behavior, try cargo vdev int stop aws then cargo vdev int start aws followed by cargo vdev int test aws. The --verbose flag can help with debugging errors. The test subcommand is supposed to be a catch all entry point (exercising the same functionality as doing start then test), but in practice, some of the container dependencies aren't as... robustly defined lets say, as they should be.

[wochinge]

I'll revert these later - this is just to speed up testing

[wochinge]

@neuronull Do you have any idea how to abstract the error from this trait? I tried with different variations and I either run into some PhantomData problem (unused generic params) or some variation of generic associated types. Both are somewhat challenging for someone doing Rust since 6 hours 🙈

[neuronull]

Is this still relevant? It looks like you overcame this in 891e119

[wochinge]

Yep, but if you know any prettier or better suggestion I'd be happy to hear them

[wochinge]

@neuronull

• Started with a cleaner implementation. So far focused upon SQS - once the refactor is done adding SNS should be a breeze. I started only with SQS to avoid fighting too many things at once.
• Happy to already get some initial feedback by you.
• Last thing missing before we can do SNS is moving out the specific error type which troubles me due to these issues

[ArzelaAscoIi]

hi @neuronull. I applied all the changes requests :)

I just pushed this commit to make make check-clippy happy. Otherwise I get this error:

Since SSSink:new is used in the sns/sqs sinks (same for the other functions/constructors) these warnings seem to be misleading. However, disabling them seems like I forgot to (maybe) link the parent method somewhere.

Running make generate-component-docs did not create new files locally. Maybe this is connected to the dead_code warning 🤔 ?

[neuronull]

Just a couple follow-ups

[neuronull]

🔨 warning: ‏Was this added by mistake? It seems like it shouldn't be necessary since we're using the new()function.

Side note- this looks like another pub that could be turned into a pub(super)

[neuronull]

🔨 warning: ‏This should actually go right above the line mod integration_tests;. The way the macro works is it gates the scope of the module it is declared over.

[neuronull]

(same)

[neuronull]

Since SSSink:new is used in the sns/sqs sinks (same for the other functions/constructors) these warnings seem to be misleading. However, disabling them seems like I forgot to (maybe) link the parent method somewhere.

Running make generate-component-docs did not create new files locally. Maybe this is connected to the dead_code warning 🤔 ?

It's quite likely related to the comment I raised about the macro to gate integration tests, I'd try moving those and then checking again.

[ArzelaAscoIi]

You were totally right. It was caused by the macro placed on top 🤦 Thanks! Currently running make generate-component-docs again. Will push the docs afterwards :)

[ArzelaAscoIi]

Done :) Should hopefully be ready. Let me know if I forgot something :)

changes made

[neuronull]

Awesome, triggering CI

[github-actions]

Regression Detector Results

Run ID: a70d1b16-6ef9-4aa1-8d38-9eeeb10e8089
Baseline: 833ac19
Comparison: 7b2bddc
Total vector CPUs: 7

Explanation

A regression test is an integrated performance test for vector in a repeatable rig, with varying configuration for vector. What follows is a statistical summary of a brief vector run for each configuration across SHAs given above. The goal of these tests are to determine quickly if vector performance is changed and to what degree by a pull request.

Because a target's optimization goal performance in each experiment will vary somewhat each time it is run, we can only estimate mean differences in optimization goal relative to the baseline target. We express these differences as a percentage change relative to the baseline target, denoted "Δ mean %". These estimates are made to a precision that balances accuracy and cost control. We represent this precision as a 90.00% confidence interval denoted "Δ mean % CI": there is a 90.00% chance that the true value of "Δ mean %" is in that interval.

We decide whether a change in performance is a "regression" -- a change worth investigating further -- if both of the following two criteria are true:

1. The estimated |Δ mean %| ≥ 5.00%. This criterion intends to answer the question "Does the estimated change in mean optimization goal performance have a meaningful impact on your customers?". We assume that when |Δ mean %| < 5.00%, the impact on your customers is not meaningful. We also assume that a performance change in optimization goal is worth investigating whether it is an increase or decrease, so long as the magnitude of the change is sufficiently large.

2. Zero is not in the 90.00% confidence interval "Δ mean % CI" about "Δ mean %". This statement is equivalent to saying that there is at least a 90.00% chance that the mean difference in optimization goal is not zero. This criterion intends to answer the question, "Is there a statistically significant difference in mean optimization goal performance?". It also means there is no more than a 10.00% chance this criterion reports a statistically significant difference when the true difference in mean optimization goal is zero -- a "false positive". We assume you are willing to accept a 10.00% chance of inaccurately detecting a change in performance when no true difference exists.

The table below, if present, lists those experiments that have experienced a statistically significant change in mean optimization goal performance between baseline and comparison SHAs with 90.00% confidence OR have been detected as newly erratic. Negative values of "Δ mean %" mean that baseline is faster, whereas positive values of "Δ mean %" mean that comparison is faster. Results that do not exhibit more than a ±5.00% change in their mean optimization goal are discarded. An experiment is erratic if its coefficient of variation is greater than 0.1. The abbreviated table will be omitted if no interesting change is observed.

No interesting changes in experiment optimization goals with confidence ≥ 90.00% and |Δ mean %| ≥ 5.00%.

Fine details of change detection per experiment.

experiment	goal	Δ mean %	Δ mean % CI	confidence
syslog_log2metric_splunk_hec_metrics	ingress throughput	+3.52	[+3.39, +3.64]	100.00%
http_to_http_noack	ingress throughput	+3.39	[+3.29, +3.49]	100.00%
enterprise_http_to_http	ingress throughput	+2.10	[+2.00, +2.19]	100.00%
otlp_http_to_blackhole	ingress throughput	+1.22	[+1.04, +1.39]	100.00%
http_to_http_acks	ingress throughput	+1.20	[-0.12, +2.53]	75.51%
syslog_loki	ingress throughput	+0.59	[+0.53, +0.64]	100.00%
datadog_agent_remap_blackhole_acks	ingress throughput	+0.59	[+0.50, +0.68]	100.00%
syslog_regex_logs2metric_ddmetrics	ingress throughput	+0.46	[+0.22, +0.71]	98.63%
datadog_agent_remap_blackhole	ingress throughput	+0.18	[+0.10, +0.25]	99.79%
splunk_hec_indexer_ack_blackhole	ingress throughput	+0.00	[-0.12, +0.12]	0.67%
splunk_hec_to_splunk_hec_logs_noack	ingress throughput	-0.00	[-0.11, +0.11]	0.25%
splunk_hec_to_splunk_hec_logs_acks	ingress throughput	-0.01	[-0.12, +0.11]	6.84%
syslog_log2metric_humio_metrics	ingress throughput	-0.02	[-0.12, +0.08]	20.83%
http_to_http_json	ingress throughput	-0.10	[-0.17, -0.04]	95.35%
http_to_s3	ingress throughput	-0.13	[-0.33, +0.07]	59.59%
syslog_humio_logs	ingress throughput	-0.27	[-0.36, -0.17]	99.98%
otlp_grpc_to_blackhole	ingress throughput	-0.29	[-0.37, -0.21]	100.00%
datadog_agent_remap_datadog_logs_acks	ingress throughput	-0.53	[-0.63, -0.42]	100.00%
fluent_elasticsearch	ingress throughput	-0.63	[-0.87, -0.40]	99.95%
splunk_hec_route_s3	ingress throughput	-1.38	[-1.79, -0.96]	100.00%
http_text_to_http_json	ingress throughput	-1.40	[-1.51, -1.30]	100.00%
syslog_splunk_hec_logs	ingress throughput	-1.44	[-1.51, -1.38]	100.00%
socket_to_socket_blackhole	ingress throughput	-1.45	[-1.49, -1.42]	100.00%
datadog_agent_remap_datadog_logs	ingress throughput	-1.56	[-1.69, -1.42]	100.00%
file_to_blackhole	egress throughput	-2.40	[-3.28, -1.53]	99.96%

[neuronull]

Thanks for your contribution @wochinge and @ArzelaAscoIi !

[ArzelaAscoIi]

Hi @neuronull. Do you know roughly how long it would take to release the SNS sink ?

[jszwedko]

Hi @neuronull. Do you know roughly how long it would take to release the SNS sink ?

The next release is targeted for the 26th. You can run one of the nightly builds if you want to try it out earlier though.