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

Feat(anomaly-connector) Log Volume Anomaly Connector #2081

Open
wants to merge 20 commits into
base: release/v1.69.0
Choose a base branch
from

Conversation

BominRahmani
Copy link
Contributor

Proposed Change

The following PR introduces the Log volume Anomaly connector.

Checklist
  • Changes are tested
  • CI has passed

@BominRahmani BominRahmani requested review from dpaasman00 and a team as code owners December 30, 2024 14:24
@BominRahmani BominRahmani force-pushed the feat/log-volume-anomaly-processor branch from bfceebb to 4ae03cf Compare January 3, 2025 18:51
@BominRahmani BominRahmani force-pushed the feat/log-volume-anomaly-processor branch from db965bf to cbff773 Compare January 8, 2025 18:21
@BominRahmani BominRahmani force-pushed the feat/log-volume-anomaly-processor branch from 82811b3 to acd6c8f Compare January 8, 2025 18:48
@antonblock antonblock changed the base branch from release/v1.68.0 to release/v1.69.0 January 8, 2025 22:08
Copy link
Contributor

@antonblock antonblock left a comment

Choose a reason for hiding this comment

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

Two quick mod changes, haven't tested yet. Please add some tests for the config and Detector

connector/loganomalyconnector/go.mod Outdated Show resolved Hide resolved
connector/loganomalyconnector/go.mod Outdated Show resolved Hide resolved
@BominRahmani BominRahmani force-pushed the feat/log-volume-anomaly-processor branch from 826f8a1 to 53da5a1 Compare January 10, 2025 06:54
MaxWindowAge time.Duration `mapstructure:"max_window_age"`
// Thresholds for anomaly detection
ZScoreThreshold float64 `mapstructure:"zscore_threshold"`
MADThreshold float64 `mapstructure:"mad_threshold"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a comment explaining what this is


// Validate checks whether the input configuration has all of the required fields for the processor.
// An error is returned if there are any invalid inputs.
func (config *Config) Validate() error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Add tests enforcing these error cases

}

// Helper to verify anomaly record
func verifyAnomalyRecord(t *testing.T, record plog.LogRecord, expectedType string, expectedRate float64) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems unused

}

// Helper to get the first log record from LogsSink
func getFirstLogRecord(t *testing.T, sink *consumertest.LogsSink) plog.LogRecord {
Copy link
Contributor

Choose a reason for hiding this comment

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

Also looks unused

// takeSample calculates and stores a new rate sample
func (d *Detector) takeSample(now time.Time) {
duration := now.Sub(d.currentBucket.start).Minutes()
if duration < (1.0 / 60.0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It might make sense to put this in a commented const to be clearer

connector/loganomalyconnector/connector.go Outdated Show resolved Hide resolved
connector/loganomalyconnector/connector.go Outdated Show resolved Hide resolved
connector/loganomalyconnector/connector.go Outdated Show resolved Hide resolved
connector/loganomalyconnector/anomaly.go Outdated Show resolved Hide resolved
connector/loganomalyconnector/connector.go Outdated Show resolved Hide resolved
connector/loganomalyconnector/connector.go Outdated Show resolved Hide resolved
connector/loganomalyconnector/factory.go Outdated Show resolved Hide resolved
connector/loganomalyconnector/connector.go Outdated Show resolved Hide resolved
connector/loganomalyconnector/connector.go Outdated Show resolved Hide resolved
connector/loganomalyconnector/connector.go Outdated Show resolved Hide resolved
connector/loganomalyconnector/connector.go Outdated Show resolved Hide resolved
connector/loganomalyconnector/anomaly.go Outdated Show resolved Hide resolved
connector/loganomalyconnector/anomaly.go Outdated Show resolved Hide resolved
connector/loganomalyconnector/connector.go Outdated Show resolved Hide resolved
@BominRahmani BominRahmani force-pushed the feat/log-volume-anomaly-processor branch from 5953b43 to 5f49322 Compare January 29, 2025 13:31
connector/loganomalyconnector/go.mod Outdated Show resolved Hide resolved
connector/loganomalyconnector/connector.go Outdated Show resolved Hide resolved
connector/loganomalyconnector/connector.go Outdated Show resolved Hide resolved
connector/loganomalyconnector/connector.go Outdated Show resolved Hide resolved
connector/loganomalyconnector/connector.go Outdated Show resolved Hide resolved
connector/loganomalyconnector/connector.go Outdated Show resolved Hide resolved
connector/loganomalyconnector/connector.go Outdated Show resolved Hide resolved
connector/loganomalyconnector/anomaly.go Outdated Show resolved Hide resolved
@BominRahmani BominRahmani force-pushed the feat/log-volume-anomaly-processor branch from ce33e15 to 56673f7 Compare February 3, 2025 14:10
@BominRahmani BominRahmani force-pushed the feat/log-volume-anomaly-processor branch from 56673f7 to 4c5a1fd Compare February 3, 2025 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants