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

fix(plugins/datadog): add return value for log function in batch queue #10044

Merged
merged 6 commits into from
Jan 6, 2023

Conversation

liverpool8056
Copy link
Contributor

@liverpool8056 liverpool8056 commented Jan 3, 2023

Summary

fix(plugins/datadog): The log function in datadog is called in batch queue when a batch is scheduled to be processed, and batch queue relys on the return value of the process it holds. The return value of log function in datadog always return nil, this makes batch queue consider the result of processing is failed. In this pr, a return value is added to fix this bug.

Checklist

Issue reference

FTI-4645

@liverpool8056
Copy link
Contributor Author

Cannot find a way to write a test case for this pr.

queue when a batch is scheduled to be processed, and batch queue relys
on the return value of the process it holds. The return value of log
function in datadog always return nil, this makes batch queue consider
the result of processing is failed. In this pr, a return value is added
to fix this bug.

FTI 4645
@liverpool8056 liverpool8056 force-pushed the plugins/add-return-value-in-datadog branch from 186a9f8 to 0ed7efe Compare January 3, 2023 15:18
Copy link
Contributor

@ADD-SP ADD-SP left a comment

Choose a reason for hiding this comment

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

LGTM

@chronolaw chronolaw changed the title add-return-value-for-log-function-in-batch-queue fix(plugins/datadog): add return value for log function in batch queue Jan 4, 2023
CHANGELOG.md Outdated Show resolved Hide resolved
@liverpool8056
Copy link
Contributor Author

Found a way to add a test case with the helps of @windmgc .

@dndx
Copy link
Member

dndx commented Jan 4, 2023

Waiting for tests as @liverpool8056 indicated.

@pull-request-size pull-request-size bot added size/S and removed size/XS labels Jan 4, 2023
@liverpool8056 liverpool8056 requested a review from dndx January 4, 2023 05:53
CHANGELOG.md Outdated
@@ -103,6 +103,8 @@
[#9877](https://github.com/Kong/kong/pull/9877)
- **JWT**: Deny requests that have different tokens in the jwt token search locations. Thanks Jackson 'Che-Chun' Kuo from Latacora for reporting this issue.
[#9946](https://github.com/Kong/kong/pull/9946)
- **Datadog**: Fix a bug that the batch queue in datadog can't get the right result when processing batch entries produced by datadog.

Choose a reason for hiding this comment

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

Let's make this text match what's in the StatsD PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@dndx dndx merged commit 276755b into master Jan 6, 2023
@dndx dndx deleted the plugins/add-return-value-in-datadog branch January 6, 2023 06:46
windmgc pushed a commit that referenced this pull request Jan 17, 2023
#10044)

The log function in Datadog is called by the batch
queue when a batch is processed, and batch queue relys
on the return value of the callback. The return value of log
function in Datadog always return `nil`, this makes batch queue consider
the result of processing as failed. In this commit, the correct return value
indicating the success in processing to fix this bug.
hanshuebner pushed a commit that referenced this pull request Jan 17, 2023
…0126)

* fix(plugins/datadog):  add return value for log function in batch queue (#10044)

The log function in Datadog is called by the batch
queue when a batch is processed, and batch queue relys
on the return value of the callback. The return value of log
function in Datadog always return `nil`, this makes batch queue consider
the result of processing as failed. In this commit, the correct return value
indicating the success in processing to fix this bug.

* fix(plugins/statsd & opentelemetry): add return values for batch queue callback functions (#10052)

Add return values for functions that will be processed in batch queue for these two plugins.
Batch queue considers the result of processing unsuccessful without a return
value, resulting the batch being reprocessed incorrectly.

FTI-4645

Co-authored-by: Robin Xiang <[email protected]>
curiositycasualty pushed a commit that referenced this pull request Oct 15, 2024
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.

6 participants