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

Jump to bottom

fix: Delete empty values iterates through nested arrays #2034

Merged
merged 3 commits into from
Dec 11, 2024
Merged

fix: Delete empty values iterates through nested arrays #2034

merged 3 commits into from
Dec 11, 2024

Conversation

justinianvoss22
Copy link
Contributor

Proposed Change

  • Added implementation for iterating through slices in telemetry
  • Cleans value depending on value in array, whether empty, map, slice, or str.
  • Add unit tests to handle

Testing

  • Unit testing
  • Test agent locally in bp to see proper processor behavior

Logs sent in

image ### Result seen in recent telemetry image

@justinianvoss22 justinianvoss22 requested review from dpaasman00 and a team as code owners December 9, 2024 21:33
dpaasman00
dpaasman00 requested changes Dec 10, 2024
View reviewed changes
Copy link
Contributor

@dpaasman00 dpaasman00 left a comment

Choose a reason for hiding this comment

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

Couple nits otherwise this is working well for me...

These attributes:
image

Get converted to this after the processor:
Screenshot 2024-12-10 at 1 31 53 PM

processor/removeemptyvaluesprocessor/processor.go Outdated Show resolved Hide resolved
processor/removeemptyvaluesprocessor/processor.go Show resolved Hide resolved
@justinianvoss22
Copy link
Contributor Author

justinianvoss22 commented Dec 10, 2024 •
edited
Loading

Couple nits otherwise this is working well for me...

These attributes: image

Get converted to this after the processor: Screenshot 2024-12-10 at 1 31 53 PM

Pretty sure we want to keep this behavior. As an array, not sure how it would be possible to make the array layout slice[0], slice[1], slice[3]. Because it is an array, I think moving up that value to the position of the deleted one makes sense.

@dpaasman00
Copy link
Contributor

Yea the behavior is correct, I was just posting that there showing it working.

dpaasman00
dpaasman00 requested changes Dec 11, 2024
View reviewed changes
Copy link
Contributor

@dpaasman00 dpaasman00 left a comment

Choose a reason for hiding this comment

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

Doesn't look like my comments were addressed, I think you just forgot to push a commit.

@justinianvoss22 justinianvoss22 force-pushed the justin/bpop-173-delete-empty-values-does-not-delete-fields-under-nested-arrays branch from 05dea42 to e926164 Compare December 11, 2024 16:14
@justinianvoss22 justinianvoss22 merged commit 6816c7f into release/v1.67.0 Dec 11, 2024
15 checks passed
@justinianvoss22 justinianvoss22 deleted the justin/bpop-173-delete-empty-values-does-not-delete-fields-under-nested-arrays branch December 11, 2024 19:55
dpaasman00 pushed a commit that referenced this pull request Dec 16, 2024
@justinianvoss22 @dpaasman00
fix: Delete empty values iterates through nested arrays (#2034)
8281820 
* delete empty values processor iterates through slices

* log body implementation

* pr review
Caleb-Hurshman added a commit that referenced this pull request Dec 18, 2024
@Caleb-Hurshman @dpaasman00
@shazlehu @mrsillydog @justinianvoss22 @djaglowski
feat: SSAPI Receiver (BPS-289) (#1951)
be7ff1b 
* chore: Update modules to v1.67.0

* fix: QRadar README typo (#2028)

Fix README typo

* fix: Shut down zombie goroutine in chronicleexporter (#2029)

* Properly shut down chronicleexporter zombie goroutine

* Fix lint

* Fix the same problem for the GRPC workflow

* ssapi mvp

* lint

* tls

* WIP

* ticker, other pr feedback

* pagination functionality

* break if results earlier than earliest_time

* fix lint

* check for earliest/latest in query

* config unit tests

* package comment

* feat(chronicleexporter): Support dynamic namespace and ingestion labels  (#1939)

* add namespace and ingenstion logs initial commit

* working except ingestion labels

* ingestion labels from attributes

* use proper log entry batch

* namespace and ingestion logs no config overwrite

* delete OverrideNamespace and OverrideIngestionLabeles

* PR changes

* fix unit tests

* modify tests again

* marshal changes

* readme and namespace check

* debug logs

* rm unnecessary clauses

* fix error wording

* rm space

* wip

* client tests

* checkpoint methods

* WIP

* functional checkpoint

* debug logs, rm print

* loadCheckpoint return error

* splunk failure test

* WIP

* encode req body

* stricter query validation

* storage config test

* lint, tidy

* return error on export fail

* tidy

* receiver tests

* receiver tests

* lint

* fix TestCheckpoint

* rename structs

* exporter fail test

* fix search checkpointing

* auth token

* lint

* fix struct name

* rm prints, fix error messages

* fix tests

* default batch size

* log end of export

* readme

* how-to

* how-to example config

* change how-to conf values

* change test batch size

* fix test case

* fix client test

* fix rebase errors

* tidy

* feat: Enforce request maximum size and number of logs (#2033)

* feat: Enforce request maximum size and number of logs

* Fix lint

* Refactor to be more go-idiomatic

* Update Chronicle exporter readme with new flags

* fix: Delete empty values iterates through nested arrays (#2034)

* delete empty values processor iterates through slices

* log body implementation

* pr review

* initial feedback

* chore: Minor cleanup of chronicle exporter names (#2046)

* chore: Save component.TelemetrySettings on chronicle exporter (#2047)

* chore: Minor cleanup of chronicle exporter names

* chore: Chronicle exporter - save component.TelemetrySettings

* safe shutdown()

* chore: Localize chronicle exporter's metrics concerns (#2048)

chore: Pull metrics-specific concerns into hostMetricsReporter

* rm err checkk from time parsing

* chore: Add debug logging (#2042)

Add debug logging

* chore: Add new tests for chronicle exporter with http and grpc servers (#2049)

* ctx check, doc notes

* chore: Rename to `bindplane-otel-collector` (#2043)

* rename to bindplane-otel-collector

* fix website links

* update report card link

* fix: Shut down zombie goroutine in chronicleexporter (#2029)

* Properly shut down chronicleexporter zombie goroutine

* Fix lint

* Fix the same problem for the GRPC workflow

* chore: Save component.TelemetrySettings on chronicle exporter (#2047)

* chore: Minor cleanup of chronicle exporter names

* chore: Chronicle exporter - save component.TelemetrySettings

* chore: Localize chronicle exporter's metrics concerns (#2048)

chore: Pull metrics-specific concerns into hostMetricsReporter

* chore: Add new tests for chronicle exporter with http and grpc servers (#2049)

* fix: Rebase cleanup (#2063)

rebase cleanup

* chore: separate http and grpc exporters (#2050)

* fix: Shut down zombie goroutine in chronicleexporter (#2029)

* Properly shut down chronicleexporter zombie goroutine

* Fix lint

* Fix the same problem for the GRPC workflow

* ssapi mvp

* initial feedback

* chore: Save component.TelemetrySettings on chronicle exporter (#2047)

* chore: Minor cleanup of chronicle exporter names

* chore: Chronicle exporter - save component.TelemetrySettings

* chore: Localize chronicle exporter's metrics concerns (#2048)

chore: Pull metrics-specific concerns into hostMetricsReporter

* chore: Add new tests for chronicle exporter with http and grpc servers (#2049)

* chore: Save component.TelemetrySettings on chronicle exporter (#2047)

* chore: Minor cleanup of chronicle exporter names

* chore: Chronicle exporter - save component.TelemetrySettings

* chore: Localize chronicle exporter's metrics concerns (#2048)

chore: Pull metrics-specific concerns into hostMetricsReporter

* chore: Add new tests for chronicle exporter with http and grpc servers (#2049)

* fix: Shut down zombie goroutine in chronicleexporter (#2029)

* Properly shut down chronicleexporter zombie goroutine

* Fix lint

* Fix the same problem for the GRPC workflow

* fix rebase stuff

---------

Co-authored-by: Dakota Paasman <[email protected]>
Co-authored-by: Sam Hazlehurst <[email protected]>
Co-authored-by: Ian Adams <[email protected]>
Co-authored-by: Justin Voss <[email protected]>
Co-authored-by: Daniel Jaglowski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dpaasman00 dpaasman00 Awaiting requested review from dpaasman00 dpaasman00 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@justinianvoss22 @dpaasman00