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

feat: add integration test for syslog exporter (and receiver) #27464

Merged

Conversation

sumo-drosiek
Copy link
Member

Description:

Adding integration tests for syslog exporter (and syslog receiver) and fixing bugs which has been found during the process

Link to tracking Issue: #21245

Testing:

Integration tests and more unit tests

Documentation:

N/A

Comment on lines +186 to +191
for i := 0; i < lrs.Len(); i++ {
lrs.At(i).SetObservedTimestamp(0)

attributes = append(attributes, lrs.At(i).Attributes().AsRaw())
lrs.At(i).Attributes().Clear()
}
Copy link
Member Author

Choose a reason for hiding this comment

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

I should perform assertions here instead of changing the received objects. Will improve it

pkg/stanza/operator/parser/syslog/syslog.go Outdated Show resolved Hide resolved
attributes map[string]interface{}
}

func TestSyslogComplementaryRFC5424(t *testing.T) {
Copy link
Member

@andrzej-stencel andrzej-stencel Oct 9, 2023

Choose a reason for hiding this comment

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

Both these tests randomly fail on my machine, apparently due to the two syslog messages arriving in different order than sent. We have investigated this with @sumo-drosiek and we believe we found the cause and the fix for this. I was actually just going to submit this fix in relation to this issue: #21244. I'm fine with merging this PR as long as the CI tests pass, I will submit the fix in a separate PR.

@@ -24,7 +24,7 @@ type CarbonDataReceiver struct {
receiver receiver.Metrics
}

// Ensure CarbonDataReceiver implements MetricDataSender.
// Ensure CarbonDataReceiver implements MetricDataReceiver.
Copy link
Member

Choose a reason for hiding this comment

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

I don't think there's such a thing as MetricDataReceiver, at least not anymore. 😉
This code seems to ensure that the CarbonDataReceiver implements the testbed.DataReceiver interface.

Copy link
Member

@andrzej-stencel andrzej-stencel left a comment

Choose a reason for hiding this comment

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

👏

Signed-off-by: Dominik Rosiek <drosiek@sumologic.com>
sumo-drosiek and others added 2 commits October 16, 2023 08:51
Co-authored-by: Daniel Jaglowski <jaglows3@gmail.com>
Signed-off-by: Dominik Rosiek <drosiek@sumologic.com>
@djaglowski djaglowski merged commit 4e3323b into open-telemetry:main Oct 16, 2023
83 checks passed
@github-actions github-actions bot added this to the next release milestone Oct 16, 2023
JaredTan95 pushed a commit to openinsight-proj/opentelemetry-collector-contrib that referenced this pull request Oct 18, 2023
…elemetry#27464)

**Description:**

Adding integration tests for syslog exporter (and syslog receiver) and
fixing bugs which has been found during the process

**Link to tracking Issue:** open-telemetry#21245 

**Testing:**

Integration tests and more unit tests

**Documentation:**

N/A

---------

Signed-off-by: Dominik Rosiek <drosiek@sumologic.com>
Co-authored-by: Daniel Jaglowski <jaglows3@gmail.com>
djaglowski pushed a commit that referenced this pull request Oct 30, 2023
**Description:**

This changes the behavior of the Syslog exporter to send each batch of
Syslog messages in a single request (with messages separated by
newlines), instead of sending each message in a separate request and
closing the connection after each message.

The batching only happens when using TCP. For UDP, each syslog message
is still sent in a separate request, as defined by [the
spec](https://datatracker.ietf.org/doc/html/rfc5426#section-3.1).

This also significantly refactors (and hopefully simplifies) the
exporter's code, extracting the code that formats the syslog messages
from the `sender` type into separate `formatter` types. Hopefully this
will make the development of this component easier.

**Link to tracking Issue:**

-
#21244

**Testing:**

The unit tests have been updated to reflect the refactored codebase. The
integration tests introduced in
#27464
are unchanged, as the format of the output messages hasn't changed.

**Documentation:**

No documentation updates.
jmsnll pushed a commit to jmsnll/opentelemetry-collector-contrib that referenced this pull request Nov 12, 2023
…elemetry#27464)

**Description:**

Adding integration tests for syslog exporter (and syslog receiver) and
fixing bugs which has been found during the process

**Link to tracking Issue:** open-telemetry#21245 

**Testing:**

Integration tests and more unit tests

**Documentation:**

N/A

---------

Signed-off-by: Dominik Rosiek <drosiek@sumologic.com>
Co-authored-by: Daniel Jaglowski <jaglows3@gmail.com>
jmsnll pushed a commit to jmsnll/opentelemetry-collector-contrib that referenced this pull request Nov 12, 2023
**Description:**

This changes the behavior of the Syslog exporter to send each batch of
Syslog messages in a single request (with messages separated by
newlines), instead of sending each message in a separate request and
closing the connection after each message.

The batching only happens when using TCP. For UDP, each syslog message
is still sent in a separate request, as defined by [the
spec](https://datatracker.ietf.org/doc/html/rfc5426#section-3.1).

This also significantly refactors (and hopefully simplifies) the
exporter's code, extracting the code that formats the syslog messages
from the `sender` type into separate `formatter` types. Hopefully this
will make the development of this component easier.

**Link to tracking Issue:**

-
open-telemetry#21244

**Testing:**

The unit tests have been updated to reflect the refactored codebase. The
integration tests introduced in
open-telemetry#27464
are unchanged, as the format of the output messages hasn't changed.

**Documentation:**

No documentation updates.
RoryCrispin pushed a commit to ClickHouse/opentelemetry-collector-contrib that referenced this pull request Nov 24, 2023
**Description:**

This changes the behavior of the Syslog exporter to send each batch of
Syslog messages in a single request (with messages separated by
newlines), instead of sending each message in a separate request and
closing the connection after each message.

The batching only happens when using TCP. For UDP, each syslog message
is still sent in a separate request, as defined by [the
spec](https://datatracker.ietf.org/doc/html/rfc5426#section-3.1).

This also significantly refactors (and hopefully simplifies) the
exporter's code, extracting the code that formats the syslog messages
from the `sender` type into separate `formatter` types. Hopefully this
will make the development of this component easier.

**Link to tracking Issue:**

-
open-telemetry#21244

**Testing:**

The unit tests have been updated to reflect the refactored codebase. The
integration tests introduced in
open-telemetry#27464
are unchanged, as the format of the output messages hasn't changed.

**Documentation:**

No documentation updates.
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.

4 participants