Skip to content

Commit

Permalink
fix: Add warn log for create/update interval (#4597)
Browse files Browse the repository at this point in the history
* fix: Add warn log for create/update interval

Add warn log for interval value from Interval and AutoEvent if the value is less than the suggested value.

Closes #4586.

Signed-off-by: Lindsey Cheng <beckysocute@gmail.com>

* fix: Modify minimum duration param type to time.Duration

Modify minimum duration param type of CheckMinInterval func to time.Duration.

Signed-off-by: Lindsey Cheng <beckysocute@gmail.com>

* fix: Remove CheckMinInterval return value and modify tests

Remove CheckMinInterval return value and verify the mock lc method is called in tests.

Signed-off-by: Lindsey Cheng <beckysocute@gmail.com>

---------

Signed-off-by: Lindsey Cheng <beckysocute@gmail.com>
  • Loading branch information
lindseysimple authored Jul 17, 2023
1 parent fa614b6 commit db7b7ee
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 1 deletion.
15 changes: 15 additions & 0 deletions internal/core/metadata/application/device.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"context"
goErrors "errors"
"fmt"
"time"

bootstrapContainer "github.com/edgexfoundry/go-mod-bootstrap/v3/bootstrap/container"
"github.com/edgexfoundry/go-mod-bootstrap/v3/di"
Expand All @@ -31,8 +32,12 @@ import (
"github.com/edgexfoundry/edgex-go/internal/core/metadata/container"
"github.com/edgexfoundry/edgex-go/internal/core/metadata/infrastructure/interfaces"
"github.com/edgexfoundry/edgex-go/internal/pkg/correlation"
"github.com/edgexfoundry/edgex-go/internal/pkg/utils"
)

// the suggested minimum duration for auto event interval
const minAutoEventInterval = 1 * time.Millisecond

// The AddDevice function accepts the new device model from the controller function
// and then invokes AddDevice function of infrastructure layer to add new device
func AddDevice(d models.Device, ctx context.Context, dic *di.Container) (id string, edgeXerr errors.EdgeX) {
Expand Down Expand Up @@ -63,6 +68,11 @@ func AddDevice(d models.Device, ctx context.Context, dic *di.Container) (id stri
correlation.FromContext(ctx),
)

// If device is successfully created, check each AutoEvent interval value and display a warning if it's smaller than the suggested 10ms value
for _, autoEvent := range d.AutoEvents {
utils.CheckMinInterval(autoEvent.Interval, minAutoEventInterval, lc)
}

deviceDTO := dtos.FromDeviceModelToDTO(addedDevice)
go publishSystemEvent(common.DeviceSystemEventType, common.SystemEventActionAdd, d.ServiceName, deviceDTO, ctx, dic)

Expand Down Expand Up @@ -162,6 +172,11 @@ func PatchDevice(dto dtos.UpdateDevice, ctx context.Context, dic *di.Container)
return errors.NewCommonEdgeXWrapper(err)
}

// If device is successfully updated, check each AutoEvent interval value and display a warning if it's smaller than the suggested 10ms value
for _, autoEvent := range device.AutoEvents {
utils.CheckMinInterval(autoEvent.Interval, minAutoEventInterval, lc)
}

lc.Debugf(
"Device patched on DB successfully. Correlation-ID: %s ",
correlation.FromContext(ctx),
Expand Down
27 changes: 27 additions & 0 deletions internal/pkg/utils/time.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
//
// Copyright (C) 2023 IOTech Ltd
//
// SPDX-License-Identifier: Apache-2.0

package utils

import (
"time"

"github.com/edgexfoundry/go-mod-core-contracts/v3/clients/logger"
)

// CheckMinInterval parses the ISO 8601 time duration string to Duration type
// and evaluates if the duration value is smaller than the suggested minimum duration
func CheckMinInterval(value string, minDuration time.Duration, lc logger.LoggingClient) {
valueDuration, err := time.ParseDuration(value)
if err != nil {
lc.Errorf("failed to parse the interval duration string %s to a duration time value: %v", value, err)
return
}

if valueDuration < minDuration {
// the duration value is smaller than the min
lc.Warnf("the interval value '%s' is smaller than the suggested value '%s', which might cause abnormal CPU increase", value, minDuration)
}
}
59 changes: 59 additions & 0 deletions internal/pkg/utils/time_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
//
// Copyright (C) 2023 IOTech Ltd
//
// SPDX-License-Identifier: Apache-2.0

package utils

import (
"errors"
"testing"
"time"

"github.com/edgexfoundry/go-mod-core-contracts/v3/clients/logger/mocks"
)

func TestCheckMinInterval(t *testing.T) {
validInterval := "1s"
lessThanMinInterval := "100us"
invalidInterval := "INVALID"
minInterval1 := 10 * time.Millisecond
minInterval2 := 1 * time.Millisecond

warnMsg := "the interval value '%s' is smaller than the suggested value '%s', which might cause abnormal CPU increase"
errMsg := "failed to parse the interval duration string %s to a duration time value: %v"
expectedErr := errors.New("time: invalid duration \"" + invalidInterval + "\"")

lcMock := &mocks.LoggingClient{}
lcMock.On("Warnf", warnMsg, validInterval, minInterval1)
lcMock.On("Warnf", warnMsg, lessThanMinInterval, minInterval2)
lcMock.On("Errorf", errMsg, invalidInterval, expectedErr)

tests := []struct {
name string
interval string
min time.Duration
logExpected bool
logLevel string
logMsg string
err error
}{
{"valid - interval is bigger than the minimum value", validInterval, minInterval1, false, "", "", nil},
{"invalid - interval is smaller than the minimum value", lessThanMinInterval, minInterval2, true, "Warnf", warnMsg, nil},
{"invalid - parsing duration string failed", invalidInterval, minInterval2, true, "Errorf", errMsg, expectedErr},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
CheckMinInterval(tt.interval, tt.min, lcMock)
if tt.logExpected {
if tt.logLevel == "Warnf" {
lcMock.AssertCalled(t, tt.logLevel, tt.logMsg, tt.interval, tt.min)
} else {
lcMock.AssertCalled(t, tt.logLevel, tt.logMsg, tt.interval, tt.err)
}
} else {
lcMock.AssertNotCalled(t, tt.logLevel, tt.logMsg, tt.interval, tt.min)
}
})
}
}
13 changes: 12 additions & 1 deletion internal/support/scheduler/application/interval.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//
// Copyright (C) 2021 IOTech Ltd
// Copyright (C) 2021-2023 IOTech Ltd
//
// SPDX-License-Identifier: Apache-2.0

Expand All @@ -8,8 +8,10 @@ package application
import (
"context"
"fmt"
"time"

"github.com/edgexfoundry/edgex-go/internal/pkg/correlation"
"github.com/edgexfoundry/edgex-go/internal/pkg/utils"
"github.com/edgexfoundry/edgex-go/internal/support/scheduler/container"
"github.com/edgexfoundry/edgex-go/internal/support/scheduler/infrastructure/interfaces"

Expand All @@ -22,6 +24,9 @@ import (
"github.com/edgexfoundry/go-mod-core-contracts/v3/models"
)

// the suggested minimum duration for scheduler interval
const minSchedulerInterval = 10 * time.Millisecond

// The AddInterval function accepts the new Interval model from the controller function
// and then invokes AddInterval function of infrastructure layer to add new Interval
func AddInterval(interval models.Interval, ctx context.Context, dic *di.Container) (id string, edgeXerr errors.EdgeX) {
Expand All @@ -41,6 +46,9 @@ func AddInterval(interval models.Interval, ctx context.Context, dic *di.Containe
addedInterval.Id,
correlation.FromContext(ctx))

// If interval is successfully created, check the interval value and display a warning if it's smaller than the suggested 10ms value
utils.CheckMinInterval(interval.Interval, minSchedulerInterval, lc)

return addedInterval.Id, nil
}

Expand Down Expand Up @@ -116,6 +124,9 @@ func PatchInterval(dto dtos.UpdateInterval, ctx context.Context, dic *di.Contain
return errors.NewCommonEdgeXWrapper(err)
}

// If interval is successfully updated, check the interval value and display a warning if it's smaller than the suggested 10ms value
utils.CheckMinInterval(interval.Interval, minSchedulerInterval, lc)

lc.Debugf(
"Interval patched on DB successfully. Correlation-ID: %s ",
correlation.FromContext(ctx),
Expand Down

0 comments on commit db7b7ee

Please sign in to comment.