Skip to content
This repository has been archived by the owner on Oct 14, 2024. It is now read-only.

Commit

Permalink
Fix issue with double counting in findings trends graph
Browse files Browse the repository at this point in the history
This commit fixes an issue where the queries building the data for the
findings trends graph accidentily counted some findings twice by
filtering on a range instead of the specific point in time shown on the
graph.

The implementation (backend and UI) for the widget has been changed to
generate and handle a list of specific times between the start and
endtimes requested instead of small ranges, and then calculate the
findings count for that specific point in time.
  • Loading branch information
Tehsmash committed May 10, 2023
1 parent 709b7f8 commit 536cf86
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 77 deletions.
20 changes: 10 additions & 10 deletions ui/src/layout/Dashboard/FindingsTrendsWidget/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,21 +47,21 @@ const TIME_RANGES = {
}
}

const TooltipHeader = ({data}) => (<div>{data.formattedEndTime}</div>)
const TooltipHeader = ({data}) => (<div>{data.formattedTime}</div>)

const WidgetChart = ({data, selectedFilters}) => {
const formattedData = data?.reduce((acc, curr) => {
const {findingType, trends} = curr;

trends.forEach(({endTime, count}) => {
const accEndTimeIndex = acc.findIndex(({endTime: accEndTime}) => endTime === accEndTime);
const formattedEndTime = formatDate(endTime);
trends.forEach(({time, count}) => {
const accTimeIndex = acc.findIndex(({time: accTime}) => time === accTime);
const formattedTime = formatDate(time);

acc = accEndTimeIndex < 0 ? [...acc, {endTime, formattedEndTime, [findingType]: count}] :
acc = accTimeIndex < 0 ? [...acc, {time, formattedTime, [findingType]: count}] :
[
...acc.slice(0, accEndTimeIndex),
{...acc[accEndTimeIndex], [findingType]: count},
...acc.slice(accEndTimeIndex + 1)
...acc.slice(0, accTimeIndex),
{...acc[accTimeIndex], [findingType]: count},
...acc.slice(accTimeIndex + 1)
];
});

Expand All @@ -73,7 +73,7 @@ const WidgetChart = ({data, selectedFilters}) => {
<ResponsiveContainer width="100%" height="100%">
<LineChart data={formattedData} margin={{top: 5, right: 0, left: 0, bottom: 60}}>
<CartesianGrid vertical={false} style={{stroke: COLORS["color-grey-lighter"]}}/>
<XAxis dataKey="formattedEndTime" tick={{fill: COLORS["color-grey"]}} style={{fontSize: "12px"}} />
<XAxis dataKey="formattedTime" tick={{fill: COLORS["color-grey"]}} style={{fontSize: "12px"}} />
<YAxis tick={{fill: COLORS["color-grey"]}} style={{fontSize: "12px"}} />
<Tooltip
content={props => <ChartTooltip {...props} widgetFindings={WIDGET_FINDINGS_ITEMS} headerDisplay={TooltipHeader} countKeyName="typeKey" />}
Expand Down Expand Up @@ -130,4 +130,4 @@ const FindingsTrendsWidget = ({className}) => {
)
}

export default FindingsTrendsWidget;
export default FindingsTrendsWidget;
7 changes: 3 additions & 4 deletions ui_backend/api/models/models.gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 2 additions & 5 deletions ui_backend/api/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -355,13 +355,10 @@ components:
readOnly: true

FindingTrend:
description: Represents the total number of findings in the time slot (between startTime and endTime).
description: Represents the total number of findings at a specific time
type: object
properties:
startTime:
type: string
format: date-time
endTime:
time:
type: string
format: date-time
count:
Expand Down
64 changes: 32 additions & 32 deletions ui_backend/api/server/server.gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

41 changes: 15 additions & 26 deletions ui_backend/pkg/rest/dashboard_findings_trends.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,6 @@ const (
timeSlotsCount = 10
)

type timeSlot struct {
StartTime, EndTime time.Time
}

func (s *ServerImpl) GetDashboardFindingsTrends(ctx echo.Context, params models.GetDashboardFindingsTrendsParams) error {
reqCtx := ctx.Request().Context()
if err := validateParams(params); err != nil {
Expand Down Expand Up @@ -90,26 +86,20 @@ func validateParams(params models.GetDashboardFindingsTrendsParams) error {
return nil
}

// createTimeSlots returns a slice of time slots (start time and end time).
// The function split the total startTime-endTime period (from the API call) into timeSlotsCount time slots,
// the finding trends will be reported based on the amount of active findings in each time slot.
func createTimeSlots(params models.GetDashboardFindingsTrendsParams) []timeSlot {
timeSlots := make([]timeSlot, timeSlotsCount)
// createTimeSlots returns a slice of points in time between endTime and startTime.
// the finding trends will be reported based on the amount of active findings in each time.
func createTimeSlots(params models.GetDashboardFindingsTrendsParams) []time.Time {
timeSlots := make([]time.Time, timeSlotsCount)
slotDuration := params.EndTime.Sub(params.StartTime) / timeSlotsCount
startTime := params.StartTime
var endTime time.Time
for i := 0; i < timeSlotsCount; i++ {
endTime = startTime.Add(slotDuration)
timeSlots[i] = timeSlot{
StartTime: startTime,
EndTime: endTime,
}
startTime = endTime
time := params.EndTime
for i := timeSlotsCount-1; i >= 0; i-- {
timeSlots[i] = time
time = time.Add(-slotDuration)
}
return timeSlots
}

func (s *ServerImpl) getFindingTrendsForFindingType(ctx context.Context, findingType models.FindingType, timeSlots []timeSlot) (models.FindingTrends, error) {
func (s *ServerImpl) getFindingTrendsForFindingType(ctx context.Context, findingType models.FindingType, timeSlots []time.Time) (models.FindingTrends, error) {
trends := make([]models.FindingTrend, len(timeSlots))
for i, slot := range timeSlots {
trend, err := s.getFindingTrendPerSlot(ctx, findingType, slot)
Expand All @@ -125,25 +115,24 @@ func (s *ServerImpl) getFindingTrendsForFindingType(ctx context.Context, finding
}, nil
}

func (s *ServerImpl) getFindingTrendPerSlot(ctx context.Context, findingType models.FindingType, slot timeSlot) (models.FindingTrend, error) {
func (s *ServerImpl) getFindingTrendPerSlot(ctx context.Context, findingType models.FindingType, slot time.Time) (models.FindingTrend, error) {
// Count total findings for the given finding type that was active during the given time slot.
findings, err := s.BackendClient.GetFindings(ctx, backendmodels.GetFindingsParams{
Count: utils.PointerTo(true),
Filter: utils.PointerTo(fmt.Sprintf(
"findingInfo/objectType eq '%s' and foundOn lt %v and (invalidatedOn eq null or invalidatedOn gt %v)",
getObjectType(findingType), slot.EndTime.Format(time.RFC3339), slot.StartTime.Format(time.RFC3339))),
"findingInfo/objectType eq '%s' and foundOn le %v and (invalidatedOn eq null or invalidatedOn gt %v)",
getObjectType(findingType), slot.Format(time.RFC3339), slot.Format(time.RFC3339))),
// Select the smallest amount of data to return in items, we only care about the count.
Select: utils.PointerTo("id"),
Top: utils.PointerTo(1),
Top: utils.PointerTo(0),
})
if err != nil {
return models.FindingTrend{}, fmt.Errorf("failed to get findings for the given slot: %v", err)
}

return models.FindingTrend{
Count: findings.Count,
StartTime: &slot.StartTime,
EndTime: &slot.EndTime,
Count: findings.Count,
Time: &slot,
}, nil
}

Expand Down

0 comments on commit 536cf86

Please sign in to comment.