Add support for streaming caching

buger commented 1 month ago

User description

Instead of creating new stream each time, cache it in memory. As cache key hash of stream config is used. E.g. if we have some dynamic consumer group, or some other variable, each will have own cache key.

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)
[ ] Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

[ ] I ensured that the documentation is up to date
[ ] I explained why this PR updates go.mod in detail with reasoning why it's required
[ ] I would like a code coverage CI quality gate exception and have explained why

PR Type

enhancement

Description

Added a caching mechanism for stream managers to improve performance by avoiding the creation of new streams for the same configuration.
Utilized sync.Map to store and retrieve cached stream managers based on a hash of the stream configuration.
Enhanced the createStreamManager function to check for existing cached stream managers before creating a new one.
Updated the Unload function to clear the stream manager cache, ensuring proper cleanup.

Changes walkthrough 📝

Relevant files

Enhancement

mw_streaming.go

Implement caching for stream managers to improve efficiency

gateway/mw_streaming.go

Added caching mechanism for stream managers using a hash of the stream
configuration.

Introduced a new sync.Map to store cached stream managers.

Modified the createStreamManager function to utilize cached stream
managers.

Updated the Unload function to clear the stream manager cache.

+26/-10

💡 PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

github-actions[bot] commented 1 month ago

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review

Error Handling
The error from json.Marshal in createStreamManager is ignored. This could lead to using an incorrect cache key if marshaling fails. Concurrency Concerns
The use of sync.Map for streamManagerCache might need further review to ensure thread safety during concurrent access, especially around the operations in createStreamManager and Unload.

github-actions[bot] commented 1 month ago

PR Code Suggestions ✨

Category	Suggestion	Score
Security	Replace MD5 with SHA-256 for generating cache keys to enhance security ___ Replace the MD5 hashing algorithm with a more secure hashing algorithm like SHA-256. MD5 is widely considered to be insecure for cryptographic purposes due to its susceptibility to collision attacks. [gateway/mw_streaming.go [151]](https://github.com/TykTechnologies/tyk/pull/6538/files#diff-6f565750150d990575c808f1ca8f38483160dc6edf05f1534cd0bedb27c2e6c8R151-R151) ```diff -cacheKey := fmt.Sprintf("%x", md5.Sum(configJSON)) +import "crypto/sha256" +... +cacheKey := fmt.Sprintf("%x", sha256.Sum256(configJSON)) ``` Suggestion importance[1-10]: 9 Why: Replacing MD5 with SHA-256 is a significant security improvement, as MD5 is known to be vulnerable to collision attacks. This change enhances the security of cache key generation.	9
Possible bug	Add error handling for JSON marshaling to prevent potential runtime issues ___ Handle the error returned by `json.Marshal` to prevent runtime panics or issues arising from unhandled errors. [gateway/mw_streaming.go [150]](https://github.com/TykTechnologies/tyk/pull/6538/files#diff-6f565750150d990575c808f1ca8f38483160dc6edf05f1534cd0bedb27c2e6c8R150-R150) ```diff -configJSON, _ := json.Marshal(streamsConfig) +configJSON, err := json.Marshal(streamsConfig) +if err != nil { + s.Logger().Errorf("Failed to marshal streams config: %v", err) + return nil +} ``` Suggestion importance[1-10]: 8 Why: Adding error handling for JSON marshaling is crucial to prevent potential runtime errors and improve the robustness of the code.	8
Enhancement	Implement structured logging for improved log management ___ Consider using structured logging for better log management and analysis. Replace `Debugf` and `Errorf` with structured logging methods. [gateway/mw_streaming.go [154-241]](https://github.com/TykTechnologies/tyk/pull/6538/files#diff-6f565750150d990575c808f1ca8f38483160dc6edf05f1534cd0bedb27c2e6c8R154-R241) ```diff -s.Logger().Debugf("Cache key: %s", cacheKey) +s.Logger().WithField("cacheKey", cacheKey).Debug("Cache key") ... -s.Logger().Errorf("Failed to marshal stream config: %v", err) +s.Logger().WithField("error", err).Error("Failed to marshal stream config") ``` Suggestion importance[1-10]: 6 Why: Implementing structured logging can improve log management and analysis, but it is not a critical change. It enhances readability and maintainability of logs.	6

TykTechnologies / tyk