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

[chore] remove converter type from stanza #36288

Merged
merged 21 commits into from
Dec 5, 2024

Conversation

bacherfl
Copy link
Contributor

Description

This PR removes the Converter type that was previously used mainly by the stanza receiver adapter (see #35669 (comment) for more details). Two other receivers were still using the converter to generate test data within the unit tests, so those have been adapted as well with this PR

Link to tracking issue

Follow up to #35453

Testing

Adapted unit tests that were still using the converter

Signed-off-by: Florian Bacher <[email protected]>
@@ -167,15 +173,6 @@ func (rt *rotationTest) Run(t *testing.T) {
fileName := filepath.Join(tempDir, "test.log")
backupFileName := filepath.Join(tempDir, "test-backup.log")

// Build expected outputs
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the generated entries for the expected log entries have not been used previously, as the require.Equal below has been commented out. I tried to make use of the plogtest.CompareLogs function to add the validation of the received log entries against the expected entries but ran into some troubles handling the order of the received log entries, which seems to be linked to the log entries being received in a random order during the log file rotation.
This however might be worth looking into as part of a separate PR dedicated to that.

@bacherfl bacherfl marked this pull request as ready for review November 12, 2024 07:44
@bacherfl bacherfl requested a review from a team as a code owner November 12, 2024 07:44
@djaglowski
Copy link
Member

Sorry for the delayed review. LGTM but now we need to resolve conflicts

@bacherfl
Copy link
Contributor Author

bacherfl commented Dec 5, 2024

Sorry for the delayed review. LGTM but now we need to resolve conflicts

no worries, and thank you for the review - will look into resolving the conflicts today

@djaglowski djaglowski merged commit 396c63d into open-telemetry:main Dec 5, 2024
158 checks passed
@github-actions github-actions bot added this to the next release milestone Dec 5, 2024
ZenoCC-Peng pushed a commit to ZenoCC-Peng/opentelemetry-collector-contrib that referenced this pull request Dec 6, 2024
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

This PR removes the `Converter` type that was previously used mainly by
the stanza receiver adapter (see
open-telemetry#35669 (comment)
for more details). Two other receivers were still using the converter to
generate test data within the unit tests, so those have been adapted as
well with this PR

<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Follow up to
open-telemetry#35453

<!--Describe what testing was performed and which tests were added.-->
#### Testing

Adapted unit tests that were still using the converter

---------

Signed-off-by: Florian Bacher <[email protected]>
sbylica-splunk pushed a commit to sbylica-splunk/opentelemetry-collector-contrib that referenced this pull request Dec 17, 2024
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

This PR removes the `Converter` type that was previously used mainly by
the stanza receiver adapter (see
open-telemetry#35669 (comment)
for more details). Two other receivers were still using the converter to
generate test data within the unit tests, so those have been adapted as
well with this PR

<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Follow up to
open-telemetry#35453

<!--Describe what testing was performed and which tests were added.-->
#### Testing

Adapted unit tests that were still using the converter

---------

Signed-off-by: Florian Bacher <[email protected]>
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.

3 participants