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.

Sign up for GitHub

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

fix: use proper key formating as per stanza persister #3879

Merged
merged 2 commits into from
Dec 12, 2023
Merged

fix: use proper key formating as per stanza persister #3879

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
44b55ef
fix: use proper key formating as per stanza persister
Nov 2, 2023
0c15a6d
fix: lint
Nov 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions cmd/migratecheckpoint/log_library.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@ type Fingerprint struct {
}

type Reader struct {
Fingerprint *Fingerprint
Offset int64
Fingerprint *Fingerprint
FileAttributes map[string]any
Copy link
Contributor

Choose a reason for hiding this comment

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

what FileAttributes doing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're trying to convert fluentd's format to the following struct as per OTeL's format. We get Offset and Fingerprint by reading the fluentd's pos_file. But the field FileAttributes is left nil.
This causes a panic when the filelogreceiver tries to access this field while decoding it (causes nil reference). Setting it to an empty map resolves this.

Screenshot 2023-11-04 at 1 28 06 AM

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense, thanks!

Offset int64
}

type journaldCursor struct {
Expand Down
11 changes: 6 additions & 5 deletions cmd/migratecheckpoint/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func (m *Migrator) MigrateContainerPos(lines []string) {
defer client.Close()
_, buf := m.ConvertFilePos(lines)

err = client.Set("$.file_input.knownFiles", buf.Bytes())
err = client.Set("file_input.knownFiles", buf.Bytes())
if err != nil {
log.Printf("error storing container checkpoints: %v", err)
}
Expand All @@ -110,7 +110,7 @@ func (m *Migrator) MigrateCustomPos(matches []string) {
if err != nil {
log.Printf("error creating a new DB client for host file checkpoints: %v", err)
}
err = client.Set("$.file_input.knownFiles", buf.Bytes())
err = client.Set("file_input.knownFiles", buf.Bytes())
Copy link
Contributor

Choose a reason for hiding this comment

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

Have we always had this issue? Is there a breaking change in stanza that occurred?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, not sure tbh. Need to dig in through commit history.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am pretty sure we have functional tests on the chart that test this. If not, we must create some or we will have more surprises.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't have any tests for migration from SCK. I'll create a PR in the chart repo covering this part.
If I understand correctly, we're moving to Golang-based functional tests, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@atoulme here's a PR to add functional test cases covering this. signalfx/splunk-otel-collector-chart#1024
PTAL! Thanks

if err != nil {
log.Printf("error storing host file checkpoints: %v", err)
}
Expand Down Expand Up @@ -141,7 +141,7 @@ func (m *Migrator) MigrateJournaldPos(matches []string) {
if err != nil {
log.Printf("error creating a new DB client for journald checkpoints: %v", err)
}
err = client.Set("$.journald_input.lastReadCursor", []byte(cursor.Cursor))
err = client.Set("journald_input.lastReadCursor", []byte(cursor.Cursor))
if err != nil {
log.Printf("error storing journald checkpoints: %v", err)
}
Expand Down Expand Up @@ -218,8 +218,9 @@ func convertToOtel(path string, hexPos string) (*Reader, error) {
}

reader := &Reader{
Fingerprint: fp,
Offset: offset,
Fingerprint: fp,
Offset: offset,
FileAttributes: map[string]any{},
}
return reader, nil
}
Loading