Skip to content
This repository has been archived by the owner on May 25, 2022. It is now read-only.

Update encoding noop #262

Merged
merged 12 commits into from
Sep 15, 2021
Merged

Conversation

Mrod1598
Copy link
Contributor

@Mrod1598 Mrod1598 commented Sep 8, 2021

Resolves #138

Added MaxLogSize to some of the build function args to allow use of it within the SplitNone function. this was to allow the function to respect max log size.

@codecov
Copy link

codecov bot commented Sep 8, 2021

Codecov Report

Merging #262 (973e47d) into main (049d0cb) will increase coverage by 0.0%.
The diff coverage is 81.2%.

Impacted file tree graph

@@          Coverage Diff          @@
##            main    #262   +/-   ##
=====================================
  Coverage   76.3%   76.3%           
=====================================
  Files         94      94           
  Lines       4434    4453   +19     
=====================================
+ Hits        3386    3401   +15     
- Misses       718     721    +3     
- Partials     330     331    +1     
Impacted Files Coverage Δ
operator/builtin/input/file/reader.go 64.0% <53.8%> (-1.6%) ⬇️
operator/builtin/input/file/config.go 77.3% <100.0%> (ø)
operator/builtin/input/file/file.go 72.2% <100.0%> (ø)
operator/builtin/input/tcp/tcp.go 74.5% <100.0%> (ø)
operator/builtin/input/udp/udp.go 71.1% <100.0%> (ø)
operator/helper/multiline.go 90.4% <100.0%> (+0.8%) ⬆️

@Mrod1598 Mrod1598 marked this pull request as ready for review September 8, 2021 20:42
@Mrod1598 Mrod1598 requested a review from a team September 8, 2021 20:42
return 0, nil, nil
}

if atEOF && len(data) == 0 {
Copy link
Member

Choose a reason for hiding this comment

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

Since if !atEOF returns, we don't need to check if atEOF

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

}

for _, tc := range testCases {
splitFunc := SplitNone(100)
Copy link
Member

Choose a reason for hiding this comment

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

I would make this a constant and tie tests to it. Equal to, slightly greater, much greater, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added more tests

Copy link
Member

Choose a reason for hiding this comment

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

This didn't really address what I was pointing out. The value 100 on this line should be a constant, because several of the tests depend on it. If we make it a constant, we can make the tests depend on the constant, which can then change without breaking tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added constant.

"simple",
[]byte("testlog1"),
func(t *testing.T, c chan *entry.Entry) {
waitForMessage(t, c, "testlog1")
Copy link
Member

Choose a reason for hiding this comment

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

A really important part of this feature is that the resulting log body should be a []byte.

Copy link
Contributor Author

@Mrod1598 Mrod1598 Sep 9, 2021

Choose a reason for hiding this comment

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

Updated to dump bytes on the emit func. Instead of decoding it, it just drops the bytes into the entry.

@djaglowski djaglowski merged commit bef2800 into open-telemetry:main Sep 15, 2021
@djaglowski djaglowski deleted the update-encoding-noop branch September 15, 2021 13:58
dmitryax added a commit to signalfx/splunk-otel-collector-chart that referenced this pull request Oct 20, 2021
dmitryax added a commit to signalfx/splunk-otel-collector-chart that referenced this pull request Oct 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support byte stream data collection
2 participants