Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

updated docs #588

Merged
merged 5 commits into from
Feb 6, 2024
Merged

updated docs #588

merged 5 commits into from
Feb 6, 2024

Conversation

aspacca
Copy link
Contributor

@aspacca aspacca commented Jan 16, 2024

Docs

What does this PR do?

Fixes #488
Fixes #521
Fixes #522

Why is it important?

Better documentation improves UX

Checklist

- [ ] My code follows the style guidelines of this project
- [ ] I have commented my code, particularly in hard-to-understand areas

  • I have made corresponding changes to the documentation
    - [ ] I have made corresponding change to the default configuration files
    - [ ] I have added tests that prove my fix is effective or that my feature works
    - [ ] I have added an entry in CHANGELOG.md

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

@aspacca aspacca self-assigned this Jan 16, 2024
Copy link

github-actions bot commented Jan 16, 2024

File Coverage Lines Branches Missing
All files 100% 100% 100%
main_aws.py 100% 100% 100%
handlers/__init__.py 100% 100% 100%
handlers/aws/__init__.py 100% 100% 100%
handlers/aws/cloudwatch_logs_trigger.py 100% 100% 100%
handlers/aws/exceptions.py 100% 100% 100%
handlers/aws/handler.py 100% 100% 100%
handlers/aws/kinesis_trigger.py 100% 100% 100%
handlers/aws/replay_trigger.py 100% 100% 100%
handlers/aws/s3_sqs_trigger.py 100% 100% 100%
handlers/aws/sqs_trigger.py 100% 100% 100%
handlers/aws/utils.py 100% 100% 100%
share/__init__.py 100% 100% 100%
share/config.py 100% 100% 100%
share/environment.py 100% 100% 100%
share/events.py 100% 100% 100%
share/expand_event_list_from_field.py 100% 100% 100%
share/factory.py 100% 100% 100%
share/include_exlude.py 100% 100% 100%
share/json.py 100% 100% 100%
share/logger.py 100% 100% 100%
share/multiline.py 100% 100% 100%
share/secretsmanager.py 100% 100% 100%
share/utils.py 100% 100% 100%
share/version.py 100% 100% 100%
shippers/__init__.py 100% 100% 100%
shippers/composite.py 100% 100% 100%
shippers/es.py 100% 100% 100%
shippers/factory.py 100% 100% 100%
shippers/logstash.py 100% 100% 100%
shippers/shipper.py 100% 100% 100%
storage/__init__.py 100% 100% 100%
storage/decorator.py 100% 100% 100%
storage/factory.py 100% 100% 100%
storage/payload.py 100% 100% 100%
storage/s3.py 100% 100% 100%
storage/storage.py 100% 100% 100%

Minimum allowed coverage is 100%

Generated by 🐒 cobertura-action against 5d48bed

@aspacca aspacca requested a review from girodav January 16, 2024 03:55
Elastic Serverless Forwarder ensures "at-least-once" delivery of the forwarded message.

The `Continuing quque` and `Replay queue` are programmatically populated by Elastic Serverless Forwarder.
This happens for the `Continuing quque` in the last two minutes of the lambda execution, if any message in the original trigger is left to be processed before that time (including the original triggers and the continuing and replay queues themselves).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This happens for the `Continuing quque` in the last two minutes of the lambda execution, if any message in the original trigger is left to be processed before that time (including the original triggers and the continuing and replay queues themselves).
This happens for the `Continuing queue` in the last two minutes of the lambda execution, if any message in the original trigger is left to be processed before that time (including the original triggers and the continuing and replay queues themselves).

Hmm does This mean the creation of the queues?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, This refers to "programmatically populated"

Comment on lines 279 to 280
It is not possible to define directly the ID of the {aws} VPC the Elastic Serverless Forwarder lambda belongs to.
The {aws} VPC is defined instead by its security group IDs and subnet IDs.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth explaining why it is not possible and add a link to the AWS docs.

As additional note, I do not think this information is enough to close out #488 as there are more pieces involved in making ESF work with PrivateLink, apart from the VPC settings (e.g VPC endpoints)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VPC endpoints are documented here

Let's move the content to this section! (and add the requirement for an EC2 VPC endpoint if cloudwatch logs are used as input)

I will add a link to https://www.elastic.co/guide/en/cloud/current/ec-traffic-filtering-vpc.html as well

do you have any hint about what @Udayel mentioned in #488?

document few important parameters like BOTO_DISABLE_COMMONNAME=False

I remember it was something related to a version bump of the ESF boto dependency, but I never hit the issue and I'm not sure if it still applies

[[at-least-once-delivery]]
== At-least-once delivery

Elastic Serverless Forwarder ensures "at-least-once" delivery of the forwarded message.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this statement should be in the overview, where the Replay queue and Continuing queue are introduced for the first time.

@@ -79,6 +79,26 @@ The forwarder can ingest logs contained within the payload of an Amazon SQS body

You can set up a separate SQS queue for each type of log. The config parameter for {es} output `es_datastream_name` is mandatory. If this value is set to an {es} data stream, the type of log must be correctly defined with configuration parameters. A single configuration file can have many input sections, pointing to different SQS queues that match specific log types.

[discrete]
[[at-least-once-delivery]]
== At-least-once delivery
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this section might be easier to digest if we split it in two, providing details for the two queues in two separate sections/sub-sections (e.g "More details about the Replay queue" or "Replay queue internals).
Each section can follow a pattern like the following:

  • What part this queue plays in ensuring at-least-once delivery
  • When ESF sends messages to this queue
  • Failures/DLQ scenarios

I would also add a note about the recent changes we made, removing sqs.deleteMessage calls and their impact.

@aspacca aspacca merged commit 9b99a3a into elastic:main Feb 6, 2024
5 checks passed
dedemorton added a commit to dedemorton/elastic-serverless-forwarder that referenced this pull request Feb 6, 2024
dedemorton added a commit to dedemorton/elastic-serverless-forwarder that referenced this pull request Feb 6, 2024
@dedemorton dedemorton mentioned this pull request Feb 6, 2024
5 tasks
dedemorton added a commit that referenced this pull request Feb 13, 2024
* Edit ESF docs added by PR #588

* Resolve open questions

* lint fix

---------

Co-authored-by: Andrea Spacca <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants