LOG-3311: Support Http output for vector #1733

vimalk78 · 2022-10-28T16:03:54Z

Description

Draft for review: API for http output

/cc
/assign

/cherry-pick

Links

Depending on PR(s):
Bugzilla:
Github issue:
JIRA: https://issues.redhat.com/browse/LOG-3311
Enhancement proposal:

openshift-ci · 2022-10-28T16:04:00Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

alanconway

Add a "method" and "maxBatch" as per https://docs.google.com/document/d/1VdeOohi70ZQdvJggxYZbxBp7vPTZ4XMVDOBh8ZYvtaw/edit#

alanconway · 2022-10-31T20:52:59Z

apis/logging/v1/output_types.go

+	//
+	// +kubebuilder:validation:Enum:=json
+	// +optional
+	Encoding string `json:"encoding,omitempty"`


Don't need this yet - we can add it later if/when we have more than one encoding.

Add a "method" and "maxBatch" as per https://docs.google.com/document/d/1VdeOohi70ZQdvJggxYZbxBp7vPTZ4XMVDOBh8ZYvtaw/edit#

Do we want maxBatch in terms of bytes, or number of events? vector supports 3 kinds
https://v0-23.vector.dev/docs/reference/configuration/sinks/http/#batch

Bytes. The limits on loki or the like are in bytes and the size of events is not predictable.

@vimalk78 does fluent support "max" bytes in this context. The choice will need to apply to both

fluentd doesn't seem to support any config parameters for batching. but recommends putting multiple records in same message with json
https://docs.fluentd.org/input/http#handle-large-data-with-batch-mode

vimalk78 · 2022-11-07T20:04:22Z

/hold

alanconway

Nearly LGTM - can you add a functional test to forward to fluentd? The first use case for this is likely to provide a workaround for lack of fluent-forward to customers forwarding to fluentd.

alanconway · 2022-12-01T16:48:53Z

test/framework/functional/output_http.go

+)
+
+const (
+	VectorImage        = "docker.io/timberio/vector:0.25.0-alpine"


Prefer quay.io image if possible. Docker rate limits can cause failures if runing tests intensively.

updated to use quay image

@alanconway you mean sending to fluentd using http?

updated PR to add a functional test which sends logs from vector to fluentd using http

jcantrill

Please also update our docs with this PR https://github.com/openshift/cluster-logging-operator/blob/master/docs/features/collection.adoc#outputs

apis/logging/v1/output_types.go

test/framework/functional/output_http.go

test/framework/functional/read.go

test/functional/outputs/http/suite_test.go

alanconway

/lgtm

openshift-ci · 2023-01-18T15:53:14Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alanconway, vimalk78

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [alanconway,vimalk78]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

vimalk78 · 2023-01-19T12:22:24Z

Please also update our docs with this PR https://github.com/openshift/cluster-logging-operator/blob/master/docs/features/collection.adoc#outputs

@jcantrill updated docs for http output

vimalk78 · 2023-01-19T12:58:36Z

/hold cancel

test/functional/outputs/http/forward_to_http_test.go

openshift-ci · 2023-01-19T20:23:47Z

@vimalk78: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-ocp-next	`ee8294e`	link	false	`/test e2e-ocp-next`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

jcantrill · 2023-01-20T16:37:22Z

/hold cancel
/lgtm

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 28, 2022

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 28, 2022

alanconway approved these changes Oct 31, 2022

View reviewed changes

vimalk78 force-pushed the log-902 branch from 8262b66 to 35e8ec8 Compare November 7, 2022 20:02

vimalk78 marked this pull request as ready for review November 7, 2022 20:04

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 7, 2022

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 7, 2022

openshift-ci bot requested review from alanconway and vparfonov November 7, 2022 20:05

vimalk78 force-pushed the log-902 branch 2 times, most recently from 680208c to ee8294e Compare November 8, 2022 18:45

vimalk78 changed the title ~~LOG-902: Support Http output~~ LOG-3311: Support Http output for vector Nov 22, 2022

alanconway suggested changes Dec 1, 2022

View reviewed changes

jcantrill added the release/5.7 label Dec 2, 2022

vimalk78 force-pushed the log-902 branch 4 times, most recently from 279a5d7 to bf69bbd Compare January 11, 2023 21:29

jcantrill requested changes Jan 12, 2023

View reviewed changes

vimalk78 force-pushed the log-902 branch from bf69bbd to 66033d5 Compare January 16, 2023 13:15

alanconway approved these changes Jan 18, 2023

View reviewed changes

openshift-ci bot assigned alanconway Jan 18, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 18, 2023

vimalk78 force-pushed the log-902 branch from 66033d5 to 6b55905 Compare January 19, 2023 12:02

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 19, 2023

vimalk78 force-pushed the log-902 branch from 6b55905 to 1ba1c5a Compare January 19, 2023 12:20

openshift-ci bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Jan 19, 2023

jcantrill reviewed Jan 19, 2023

View reviewed changes

test/functional/outputs/http/forward_to_http_test.go Outdated Show resolved Hide resolved

vimalk78 force-pushed the log-902 branch from 1ba1c5a to 27d756a Compare January 19, 2023 19:08

LOG-902: Support Http output

db03078

vimalk78 force-pushed the log-902 branch from 27d756a to db03078 Compare January 19, 2023 20:22

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 20, 2023

openshift-ci bot assigned jcantrill Jan 20, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 20, 2023

vimalk78 requested review from jcantrill and removed request for vparfonov January 20, 2023 20:35

openshift-merge-robot merged commit 55dfc13 into openshift:master Jan 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LOG-3311: Support Http output for vector #1733

LOG-3311: Support Http output for vector #1733

vimalk78 commented Oct 28, 2022 •

edited

openshift-ci bot commented Oct 28, 2022

alanconway left a comment

alanconway Oct 31, 2022

vimalk78 Nov 1, 2022

alanconway Nov 2, 2022

jcantrill Nov 2, 2022

vimalk78 Jan 12, 2023

vimalk78 commented Nov 7, 2022

alanconway left a comment

alanconway Dec 1, 2022

vimalk78 Jan 11, 2023

vimalk78 Jan 11, 2023

vimalk78 Jan 11, 2023

jcantrill left a comment

alanconway left a comment

openshift-ci bot commented Jan 18, 2023

vimalk78 commented Jan 19, 2023

vimalk78 commented Jan 19, 2023

openshift-ci bot commented Jan 19, 2023 •

edited

jcantrill commented Jan 20, 2023

LOG-3311: Support Http output for vector #1733

LOG-3311: Support Http output for vector #1733

Conversation

vimalk78 commented Oct 28, 2022 • edited

Description

Links

openshift-ci bot commented Oct 28, 2022

alanconway left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vimalk78 commented Nov 7, 2022

alanconway left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcantrill left a comment

Choose a reason for hiding this comment

alanconway left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Jan 18, 2023

vimalk78 commented Jan 19, 2023

vimalk78 commented Jan 19, 2023

openshift-ci bot commented Jan 19, 2023 • edited

jcantrill commented Jan 20, 2023

vimalk78 commented Oct 28, 2022 •

edited

openshift-ci bot commented Jan 19, 2023 •

edited