• app/apphandlers/embeddedAppHandler.go (1 hunks)
• app/apphandlers/processorAppHandler.go (1 hunks)
• app/cluster/integration_test.go (1 hunks)
• gateway/webhook/webhook_test.go (1 hunks)
• mocks/router/transformer/mock_transformer.go (1 hunks)
• processor/integrations/integrations.go (1 hunks)
• processor/processor.go (1 hunks)
• processor/processor_test.go (1 hunks)
• router/factory.go (2 hunks)
• router/handle.go (4 hunks)
• router/handle_lifecycle.go (4 hunks)
• router/manager/manager_test.go (2 hunks)
• router/router_isolation_test.go (3 hunks)
• router/router_test.go (15 hunks)
• router/transformer/transformer.go (8 hunks)
• router/transformer/transformerProxyAdapter.go (1 hunks)
• router/transformer/transformerProxyAdapter_test.go (1 hunks)
• router/transformer/transformer_test.go (11 hunks)
• router/types/types.go (1 hunks)
• router/utils/utils.go (1 hunks)
• router/worker.go (11 hunks)
• router/worker_test.go (1 hunks)
• services/transformer/features.go (2 hunks)
• services/transformer/features_impl.go (1 hunks)

• processor/integrations/integrations.go
• processor/processor.go
• processor/processor_test.go

app/apphandlers/embeddedAppHandler.go (1)

• 277-289: The addition of the TransformerFeaturesService field to the router.Factory struct is consistent with the changes described in the summary. Ensure that the transformerFeaturesService is properly initialized before being used here, which seems to be the case as per the code above the hunk.

app/apphandlers/processorAppHandler.go (2)

• 267-279: The addition of the TransformerFeaturesService field to the router.Factory struct aligns with the changes described in the summary. Ensure that the TransformerFeaturesService is properly initialized before being used in the router.Factory.

• 265-281: No further issues found in the context of the provided hunk. The rest of the file appears to be unchanged and relevant to the update.

app/cluster/integration_test.go (1)

• 229-229: The addition of TransformerFeaturesService to the router.Factory struct is consistent with the pull request's goal of integrating a new transformer features service. Ensure that the NewNoOpService is the intended implementation for the integration test and that the actual service is correctly initialized in the production code.

gateway/webhook/webhook_test.go (2)

• 86-88: The implementation of TransformerProxyVersion in mockTransformerService returns a hardcoded string "random-version". Ensure that this is the intended behavior for tests and that it aligns with the expected versioning logic.

• 83-92: No further issues found in the provided hunk. The rest of the file appears to be unchanged and consistent with the expected behavior described in the pull request summary.

mocks/router/transformer/mock_transformer.go (1)

• 40-44: The ProxyRequest method in MockTransformer has been correctly updated to return a pointer to transformer.ProxyRequestResponse. Ensure that all usages of this mock in tests are updated to work with the new return type.

router/factory.go (3)

• 17-29: The addition of the TransformerFeaturesService field to the Factory struct aligns with the summary and is correctly implemented.

• 43-49: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [31-49]

The New function correctly passes the TransformerFeaturesService to the Setup method of the Handle struct, as per the changes described in the summary.

• 31-31: The summary mentions that f.TransformerFeaturesService is passed as an argument to the New function, but the code shows it's being used within the function. Please verify if the summary is accurate or if the function signature should be updated to include f.TransformerFeaturesService as a parameter.

router/handle.go (4)

• 25-30: The removal of the "integrations" import and the addition of the "transformer" import are consistent with the changes in the codebase.

• 39-45: The addition of the "transformerFeaturesService" import aligns with the new functionality being integrated into the codebase.

• 48-58: The addition of the transformerFeaturesService to the Handle struct aligns with the new functionality being integrated into the codebase.

• 619-671: The changes to the function handleOAuthDestResponse, including the additional parameter and the updated return type, are consistent with the new functionality for handling OAuth destination responses.

router/handle_lifecycle.go (4)

• 32-32: The import of the transformer features service is correctly added to support the new functionality.

• 48-48: The addition of the transformerFeaturesService to the Setup function signature is correct and aligns with the summary.

• 60-60: The assignment of transformerFeaturesService to rt.transformerFeaturesService in the Setup function is correct and necessary for the new functionality.

• 319-327: The addition of waiting for transformer features in the Start function is implemented correctly and includes appropriate logging.

router/manager/manager_test.go (2)

• 29-34: The import for the transformer service has been correctly added to support the new TransformerFeaturesService field in the router.Factory struct.

• 198-209: The TransformerFeaturesService field has been correctly initialized with transformer.NewNoOpService(). Ensure that this is the intended service for the test environment and that it behaves as expected during tests.

router/router_isolation_test.go (4)

• 34-34: The import statement for the destination package has been added to support the setup of a transformer container in the test environment.

• 181-182: The SetupTransformer function is called and the error is checked properly, ensuring that the transformer container is set up without issues.

• 216-216: The configuration for the transformer URL is set correctly after ensuring the transformer container is set up without errors.

• 213-220: The configuration values for the database and other settings are set correctly for the test environment.

router/router_test.go (15)

• 37-40: The import for transformerFeaturesService is correctly added to support the new features.

• 338-341: The router.Setup function call has been correctly updated to include the transformerFeaturesService.NewNoOpService() as an argument.

• 355-358: The router.Setup function call in the test case has been correctly updated to include the transformerFeaturesService.NewNoOpService() as an argument.

• 446-449: The router.Setup function call in the test case has been correctly updated to include the transformerFeaturesService.NewNoOpService() as an argument.

• 533-536: The router.Setup function call in the test case has been correctly updated to include the transformerFeaturesService.NewNoOpService() as an argument.

• 611-614: The router.Setup function call in the test case has been correctly updated to include the transformerFeaturesService.NewNoOpService() as an argument.

• 690-693: The router.Setup function call in the test case has been correctly updated to include the transformerFeaturesService.NewNoOpService() as an argument.

• 791-794: The router.Setup function call in the test case has been correctly updated to include the transformerFeaturesService.NewNoOpService() as an argument.

• 888-891: The router.Setup function call in the test case has been correctly updated to include the transformerFeaturesService.NewNoOpService() as an argument.

• 1019-1022: The router.Setup function call in the test case has been correctly updated to include the transformerFeaturesService.NewNoOpService() as an argument.

• 1118-1121: The router.Setup function call in the test case has been correctly updated to include the transformerFeaturesService.NewNoOpService() as an argument.

• 1263-1266: The router.Setup function call in the test case has been correctly updated to include the transformerFeaturesService.NewNoOpService() as an argument.

• 1440-1443: The router.Setup function call in the test case has been correctly updated to include the transformerFeaturesService.NewNoOpService() as an argument.

• 1650-1653: The router.Setup function call in the test case has been correctly updated to include the transformerFeaturesService.NewNoOpService() as an argument.

• 1818-1821: The router.Setup function call in the test case has been correctly updated to include the transformerFeaturesService.NewNoOpService() as an argument.

router/transformer/transformer.go (9)

• 22-27: The import of router_utils has been removed. Ensure that there are no references to it in the codebase that would cause compilation errors.

• 65-66: The Metadata field in ProxyRequestPayload struct has been correctly updated to be a slice of ProxyRequestMetadata.

• 70-72: The Adapter field has been correctly added to the ProxyRequestParams struct.

• 75-82: The new ProxyRequestResponse struct has been correctly defined with all the necessary fields.

• 87-87: The ProxyRequest method in the Transformer interface now correctly returns a pointer to ProxyRequestResponse.

• 252-316: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [255-351]

The ProxyRequest method includes comprehensive error handling and logging for various scenarios, which is a good practice for debugging and observability.

• 385-385: The doProxyRequest method has been correctly updated to accept proxyUrl and destName as parameters.

• 413-419: When a timeout occurs, the status code is set to http.StatusGatewayTimeout, which is appropriate for indicating that a gateway or proxy timed out.

• 455-461: When there is an error reading from resp.Body, the status code is set to http.StatusInternalServerError. This is appropriate as it indicates a server-side issue with reading the response.

router/transformer/transformerProxyAdapter.go (4)

• 3-3: The use of go:generate for mock generation is a good practice for maintaining testability.

• 15-19: The introduction of the transformerProxyAdapter interface is a good practice for abstraction and will facilitate easier testing and future extensions.

• 141-143: Verify that the DEST_TRANSFORM_URL configuration is set correctly in the environment where this code will run to ensure proper URL construction.

• 84-87: Ensure that the job IDs in the metadata are unique to prevent any potential overwriting of response codes and bodies in the routerJobResponseCodes and routerJobResponseBodys maps within the getResponse function of v0Adapter.

router/transformer/transformer_test.go (4)

• 6-12: The import of "net/url" is necessary for the mockAdapter implementation, which uses url.JoinPath.

• 53-57: The constant EXACT_STR is defined but not used in any test case.

• 133-137: The Metadata field in ProxyRequestPayload is correctly populated with static values for testing.

• 347-351: The ProxyRequestParams struct is correctly populated with a mockAdapter instance for testing the ProxyRequest method.

router/types/types.go (1)

• 78-81: The addition of the DontBatch field to the JobMetadataT struct is consistent with the summary and the pull request's goal of handling partial failures. Ensure that the logic that processes job metadata correctly interprets this new field where batching decisions are made.

router/utils/utils.go (1)

• 73-73: The addition of the DontBatch field to the JobParameters struct aligns with the pull request's goal to handle job metadata updates for partial failure handling.

router/worker.go (11)

• 172-175: The addition of the DontBatch field in the JobMetadataT struct is correctly implemented and is being used in the worker logic to determine if a job should be batched or not.

• 628-809: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [306-813]

The refactored logic for processing destination jobs, including handling response codes and bodies, appears to be complex. Ensure that the logic is thoroughly tested, especially the conditions for batching and the handling of different response codes.

• 628-809: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [306-813]

The changes to the processDestinationJobs method are extensive and introduce new logic for handling partial failures. It is crucial to ensure that these changes are well-tested, especially the new branching logic and the handling of different response scenarios.

• 631-667: The new helper functions setDontBatchDirectives, consolidateRespBodys, and anyNonTerminatedCode are added to support the updated logic in processDestinationJobs. Ensure that these functions are used correctly and that their logic aligns with the intended behavior of the worker.

• 670-715: The proxyRequest method handles the logic for making proxy requests. Ensure that this method correctly handles all possible error scenarios and edge cases, especially since it interacts with external services.

• 818-829: The logic for updating request metrics in the updateReqMetrics method appears to be correctly implemented. However, it is important to verify that the metrics are updated accurately and reflect the actual behavior of the worker.

• 628-809: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [306-813]

The processDestinationJobs method has been updated with new logic that handles shared resources. Ensure that there are no data races, especially when accessing or modifying shared maps like failedJobOrderKeys and dontBatchDirectives.

• 670-715: The logic for handling OAuth token fetching and error handling in the proxyRequest method should be reviewed to ensure that OAuth errors are handled securely and that sensitive information is not exposed.

• 789-798: The prepareResponsesForJobs method has been updated to handle job retries and aborted jobs. Ensure that the logic for setting job states and retry times aligns with the intended business logic and that jobs are not retried or aborted incorrectly.

• 628-809: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [800-816]

The logic for sending event delivery statistics and responses to the config backend should be reviewed to ensure that the data sent is accurate and that the logic for determining which events to send is correct.

• 390-398: The logic for tracking stuck deliveries in the processDestinationJobs method should be reviewed to ensure that it does not lead to false positives or negatives, and that it accurately reflects the state of deliveries.

router/worker_test.go (4)

• 32-98: The test TestSetDontBatchDirectives appears to be correctly structured and tests various scenarios for the setDontBatchDirectives function.

• 100-163: The test TestConsolidateRespBodys is well-structured and tests the consolidateRespBodys function with different input scenarios.

• 165-213: The test TestAnyNonTerminatedCode is properly implemented to test the anyNonTerminatedCode function with various response code maps.

• 215-467: The test suite for Proxy Request is comprehensive and covers different response scenarios from the proxyRequest function. It's important to ensure that the mock expectations set up for ProxyRequest calls on the mockTransformer are aligned with the actual behavior of the transformer.ProxyRequest function in production.

services/transformer/features.go (3)

• 23-28: The addition of TransformerProxyVersion() to the FeaturesService interface is consistent with the summary provided. Ensure that all implementations of this interface have been updated to include this new method.

• 60-62: The implementation of TransformerProxyVersion() in noopService correctly returns V0, which seems to be the default or no-operation version. This is in line with the expected behavior for a no-op service.

• 64-66: The Wait() method in noopService provides a non-blocking channel, which is appropriate for a no-op implementation.

services/transformer/features_impl.go (1)

• 33-39: The implementation of TransformerProxyVersion looks correct and follows best practices.

• router/transformer/transformer.go (8 hunks)
• router/transformer/transformerProxyAdapter_test.go (1 hunks)
• router/worker.go (11 hunks)
• router/worker_test.go (1 hunks)

router/transformer/transformer.go (1)

• 436-437: Skip this comment as the use of http.StatusInternalServerError instead of http.StatusNotFound is intentional as per previous discussions.

router/worker.go (15)

• 172-175: The addition of the DontBatch field in the JobMetadataT struct is correctly implemented and used within the processDestinationJobs function.

• 306-314: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [306-348]

The logic in the processDestinationJobs function correctly handles the new response status codes and bodies, ensuring that individual jobs are processed appropriately.

• 681-727: The proxyRequest function has been updated to return response status codes, response bodies, and content type, aligning with the new requirements for handling responses.

• 729-735: The hydrateRespStatusCodes function correctly populates response status codes and bodies for all jobs, ensuring that no job is left without a response.

• 749-797: The prepareRouterJobResponses function correctly handles response status codes and bodies for individual jobs and prepares router job responses accordingly.

• 800-808: The prepareResponsesForJobs function is correctly implemented to prepare response status codes and bodies for jobs.

• 630-820: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [811-826]

The canSendJobToDestination function correctly checks failed job order keys to determine if a job can be sent to the destination.

• 829-840: The updateReqMetrics function correctly handles response status codes for request metrics, ensuring accurate tracking of request success and retries.

• 826-843: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [843-850]

The allowRouterAbortedAlert function correctly determines whether to alert on aborted events based on the errorAt parameter.

• 172-178: > Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [850-859]

The updateAbortedMetrics function correctly updates metrics for aborted events with the appropriate tags and alerting behavior.

• 172-178: > Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [850-911]

The postStatusOnResponseQ function correctly posts job status to the response queue, enhancing the error response with additional information and handling success and failure cases appropriately.

• 172-178: > Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [911-928]

The sendRouterResponseCountStat function correctly tracks router response counts with the necessary tags, including alerting behavior and error location.

• 172-178: > Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [928-953]

The sendEventDeliveryStat function correctly tracks event delivery metrics with the necessary tags, including delivery time when available.

• 172-178: > Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [953-972]

The sendDestinationResponseToConfigBackend function correctly sends destination response to the config backend, including all necessary information for tracking.

• 172-178: > Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [972-1000]

The retryLimitReached function correctly determines if the retry limit for a job has been reached, taking into account status codes and time windows.

router/worker_test.go (5)

• 32-98: The test TestSetDontBatchDirectives appears to be well-structured and tests various scenarios for the setDontBatchDirectives function.

• 100-163: The test TestConsolidateRespBodys is well-structured and tests the consolidateRespBodys function with different input scenarios.

• 165-220: The test TestAnyNonTerminalCode correctly tests the anyNonTerminalCode function with various input scenarios to ensure it behaves as expected.

• 222-277: The test TestAllNonTerminalCode is well-structured and tests the allNonTerminalCodes function with different input scenarios, ensuring it behaves as expected.

• 279-530: The test suite for Proxy Request is comprehensive and covers different scenarios, including successful proxy requests, non-200 responses, and OAuth handling. It's important to ensure that the mock expectations set up for ProxyRequest calls in the tests are aligned with the actual behavior of the ProxyRequest method after the recent changes.

• router/transformer/transformerProxyAdapter.go (1 hunks)
• router/transformer/transformerProxyAdapter_test.go (1 hunks)

router/transformer/transformerProxyAdapter_test.go (2)

• 15-102: The tests for V0Adapter and V1Adapter are comprehensive and cover the functionality well. They check for correct URL generation, payload creation, and response parsing, including both success and error scenarios. The use of hardcoded strings for expected payloads and responses is acceptable in this context to ensure the output matches expectations exactly. Good use of the require package for assertions and fmt.Sprintf for dynamic URL construction. Overall, the tests are well-structured and maintainable.

• 105-207: No issues found in the V1Adapter tests. They follow the same good practices as the V0Adapter tests, including checking for correct URL generation, payload creation, and response parsing. The tests are comprehensive, covering both success and error scenarios, and are well-structured for maintainability.