Skip to content

pipeline: outputs: cloudwatch: document the tls option - #2840

Open
cvas22 wants to merge 2 commits into
fluent:masterfrom
cvas22:cloudwatch-tls
Open

cvas22 wants to merge 2 commits into
fluent:masterfrom
cvas22:cloudwatch-tls

Conversation

@cvas22

@cvas22 cvas22 commented Nov 12, 2026 •

Copy link
Copy Markdown

Docs for fluent/fluent-bit#12602, which makes cloudwatch_logs honor tls. It used to stay on TLS whatever you set.

This adds tls to the parameter table (default on) and sets tls: off in the LocalStack example, since that endpoint is plain HTTP and the example wouldn't connect otherwise.

Summary by CodeRabbit

  • Documentation
    • Documented the CloudWatch Logs TLS setting, including how to use plain-HTTP endpoints. Both LocalStack examples now show TLS turned off.

Signed-off-by: Srinivas Chippagiri <cvas22@gmail.com>
@coderabbitai

coderabbitai Bot commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Important

Review skipped

We couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting @coderabbitai full review.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review

No actionable comments were generated in the recent review.

Walkthrough

The CloudWatch Logs output reference now documents a tls option that defaults to on and can be set to off for plain-HTTP endpoints. Both LocalStack examples set TLS off.

Changes: CloudWatch TLS documentation

Layer / File(s) Summary
TLS option and LocalStack examples
pipeline/outputs/cloudwatch.md
The configuration table documents the tls option, its default, and its use with plain-HTTP endpoints. Both LocalStack examples set TLS off.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Other

Merge Risk: Minimal · previous review 5bfc3

This documentation-only change adds the TLS option and sets it to off in the LocalStack examples. It matches the targeted upcoming release and carries no meaningful merge risk.

Pre-merge checks: Passed 4

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title names the file area (pipeline/outputs/cloudwatch) and the change (documenting the tls option). The raw summary confirms the PR adds a tls option entry to the CloudWatch Logs output reference…
Full details: Title check

Explanation

The title names the file area (pipeline/outputs/cloudwatch) and the change (documenting the tls option). The raw summary confirms the PR adds a tls option entry to the CloudWatch Logs output reference, with default on and off for plain-HTTP endpoints, and sets tls off in both LocalStack examples. The title matches the main change. It uses a 'path-style' prefix format, which is acceptable and still specific.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 688968af-a694-4643-995c-3b417ace79f7
Commits

Reviewing files that changed from the base of the PR and between 75f6d7a and 5bfc3f2.

Files selected for processing: 1

  • pipeline/outputs/cloudwatch.md

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

@eschabell eschabell self-assigned this Nov 14, 2026
@eschabell eschabell added waiting-on-code-merge waiting-on-review Waiting on a review from mainteners labels Nov 14, 2026
@eschabell eschabell changed the title outputs: cloudwatch: document the tls option pipeline: outputs: cloudwatch: document the tls option Nov 14, 2026
    - Add the port parameter to the table (default 443). The LocalStack
      example already uses it.
    - Say in the tls row that a plain HTTP endpoint also needs port set,
      because the port stays 443 when tls is off.
    - Pad both rows to the table's existing column widths.

Signed-off-by: Eric D. Schabell <eric@schabell.org>

@eschabell eschabell left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@cvas22 thanks for the docs PR, really appreciate it! I've added a small follow on adjustment but it's ready to go. There is one issue I'm bookmarking here for action when the code PR merges, so no action needed now from you!

Still open before merge this PR

  • The tls row needs a version qualifier ("In vX or later"). Released versions ignore tls on this plugin, and tls on fails with "does not support TLS". I left it out
    because #12602 has no milestone yet. Once the target release is known, add the sentence and the matching version label here.

@eschabell eschabell removed the waiting-on-review Waiting on a review from mainteners label Nov 14, 2026
@eschabell

Copy link
Copy Markdown
Collaborator

Follow-up commit b2a5715 adds a port row and updates the tls row.

With tls: off, a plain HTTP endpoint usually also needs a non-default port, such as 4566 for LocalStack. The plugin already reads port (default 443) and passes it to the upstream connection. That option has existed since v2.1.0 (fluent/fluent-bit@9efd3532a) but was never in the parameter table. The tls row now tells readers to set port along with tls: off.

The port row doesn't depend on fluent/fluent-bit#12602. The tls row still does, so this PR keeps waiting on that merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants