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

Fixes #20607: Make possible to choose precision in compliance percent API #4127

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# SPDX-License-Identifier: CC-BY-SA-2.0
# SPDX-FileCopyrightText: 2013-2020 Normation SAS
name: "precision"
in: query
schema:
type: integer
example: 0
default: 2
description: Number of digits after comma in compliance percent figures
Copy link
Member

Choose a reason for hiding this comment

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

we can maybe precise here that a low value will never be zero and that is it not a simple rounding.

2 changes: 2 additions & 0 deletions webapp/sources/api-doc/paths/compliance/global.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ get:
summary: Global compliance
description: Get current global compliance of a Rudder server
operationId: getGlobalCompliance
parameters:
- $ref: ../../components/parameters/compliance-percent-precision.yml
responses:
"200":
description: Success
Expand Down
1 change: 1 addition & 0 deletions webapp/sources/api-doc/paths/compliance/node.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ get:
operationId: getNodeCompliance
parameters:
- $ref: ../../components/parameters/compliance-level.yml
- $ref: ../../components/parameters/compliance-percent-precision.yml
- $ref: ../../components/parameters/node-id.yml
responses:
"200":
Expand Down
1 change: 1 addition & 0 deletions webapp/sources/api-doc/paths/compliance/nodes.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ get:
operationId: getNodesCompliance
parameters:
- $ref: ../../components/parameters/compliance-level.yml
- $ref: ../../components/parameters/compliance-percent-precision.yml
responses:
"200":
description: Success
Expand Down
1 change: 1 addition & 0 deletions webapp/sources/api-doc/paths/compliance/rule.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ get:
operationId: getRuleCompliance
parameters:
- $ref: ../../components/parameters/compliance-level.yml
- $ref: ../../components/parameters/compliance-percent-precision.yml
- $ref: ../../components/parameters/rule-id.yml
responses:
"200":
Expand Down
1 change: 1 addition & 0 deletions webapp/sources/api-doc/paths/compliance/rules.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ get:
operationId: getRulesCompliance
parameters:
- $ref: ../../components/parameters/compliance-level.yml
- $ref: ../../components/parameters/compliance-percent-precision.yml
responses:
"200":
description: Success
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,11 +245,7 @@ object Doobie {
)
}

implicit val CompliancePercentRead: Read[CompliancePercent] = {
import ComplianceLevelSerialisation._
import net.liftweb.json._
Read[String].map(json => parsePercent(parse(json)))
}

implicit val CompliancePercentWrite: Write[CompliancePercent] = {
import ComplianceLevelSerialisation._
import net.liftweb.json._
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
package com.normation.rudder.domain.reports

import com.normation.rudder.domain.logger.ComplianceLogger
import com.normation.rudder.domain.reports.ComplianceLevel.PERCENT_PRECISION

import net.liftweb.http.js.JE
import net.liftweb.http.js.JE.JsArray
Expand Down Expand Up @@ -87,7 +88,7 @@ final case class CompliancePercent(
, nonCompliant : Double = 0
, auditError : Double = 0
, badPolicyMode : Double = 0
) {
)(val precision: Int = 0) {
val compliance = success+repaired+notApplicable+compliant+auditNotApplicable
}

Expand All @@ -100,40 +101,42 @@ object CompliancePercent {
, Disabled, AuditCompliant, AuditNotApplicable, AuditNonCompliant, AuditError, BadPolicyMode).map(_.level)
}
{ // maintenance sanity check between dimension
List(ComplianceLevel(), CompliancePercent()).foreach { inst =>
List(ComplianceLevel(), CompliancePercent()()).foreach { inst =>
// product arity only compare first arg list
if(inst.productArity != WORSE_ORDER.length) {
throw new IllegalArgumentException(s"Inconsistency in ${inst.getClass.getSimpleName} code (checking consistency of" +
s" fields for WORSE_ORDER). Please report to a developer, this is a coding error")
}
}
}

// the value for the minimum percent reported even if less (see class description)
val MIN_PC = BigDecimal(0.01) // since we keep two digits in percents

/*
* Ensure that the sum is 100% and that no case disapear because it is
* less that 0.01%.
* less that 0.01% (assuming precision 2).
*
* Rules are:
* - always keeps at least 0.01% for any case
* (given that we have 14 categories, it means that at worst, if 13 are at 0.1 in place of ~0%;
* the last one will be false by 0.13%, which is ok)
* - always keeps at least 10e(-precision)% for any case
* (given that we have 14 categories, it means that at worst, if 13 are at the mean in place of ~0%;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* (given that we have 14 categories, it means that at worst, if 13 are at the mean in place of ~0%;
* (given that we have 14 categories, it means that at worst, if 13 are at the min in place of ~0%;

* the last one will be false by 13*10e(-precision)%. For precision = 0, it can reach 13% in a pathological case.
* - the biggest value is rounded to compensate
* - in case of equality (ex: 1/3 success, 1/3 NA, 1/3 error), the biggest is
* chosen accordingly to "ReportType.level" logic.
*
*/
def fromLevels(c: ComplianceLevel): CompliancePercent = {
def fromLevels(c: ComplianceLevel, precision: Int): CompliancePercent = {
// the value for the minimum percent reported even if less (see class description)
val MIN_PC = BigDecimal(1) * BigDecimal(10).pow(- precision) // since we keep precision digits in percents

if(c.total == 0) { // special case: let it be 0
CompliancePercent()
CompliancePercent()(precision)
} else {
val total = c.total // not zero
def pc_for(i:Int) : Double = {
if(i == 0) {
0
} else {
val pc = (i * 100 / BigDecimal(total)).setScale(2, BigDecimal.RoundingMode.HALF_UP)
val pc = (i * 100 / BigDecimal(total)).setScale(precision, BigDecimal.RoundingMode.HALF_UP)
(if(pc < MIN_PC) MIN_PC else pc).toDouble
}
}
Expand All @@ -155,7 +158,7 @@ object CompliancePercent {
val all = ((pc_last, levels.last._2) :: pc).sortBy(_._2).map(_._1)

// create array of pecents
CompliancePercent.fromSeq(all)
CompliancePercent.fromSeq(all, precision)
}
}

Expand Down Expand Up @@ -203,13 +206,13 @@ object CompliancePercent {
* Init compliance percent from a Seq. Order is of course extremely important here.
* This is a dangerous internal only method: if the seq is too short or too big, throw an error.
*/
protected def fromSeq(pc: Seq[Double]) = {
protected def fromSeq(pc: Seq[Double], precision: Int) = {
val expected = WORSE_ORDER.length
if(pc.length != expected) {
throw new IllegalArgumentException(s"We are trying to build a compliance bar from a sequence of double that has" +
s" not the expected length of ${expected}, this is a code bug, please report it: + ${pc}")
} else {
CompliancePercent(pc(0), pc(1), pc(2), pc(3), pc(4), pc(5), pc(6), pc(7), pc(8), pc(9), pc(10), pc(11), pc(12), pc(13))
CompliancePercent(pc(0), pc(1), pc(2), pc(3), pc(4), pc(5), pc(6), pc(7), pc(8), pc(9), pc(10), pc(11), pc(12), pc(13))(precision)
}
}
}
Expand Down Expand Up @@ -240,11 +243,8 @@ final case class ComplianceLevel(
lazy val total = pending+success+repaired+error+unexpected+missing+noAnswer+notApplicable+reportsDisabled+compliant+auditNotApplicable+nonCompliant+auditError+badPolicyMode
lazy val total_ok = success+repaired+notApplicable+compliant+auditNotApplicable

lazy val pc = CompliancePercent.fromLevels(this)
// compliance excluding disabled and pending reports
lazy val complianceWithoutPending = CompliancePercent.fromLevels(this.copy(pending = 0, reportsDisabled = 0)).compliance
lazy val compliance = pc.compliance

def withoutPending = this.copy(pending = 0, reportsDisabled = 0)
def computePercent(precision: Int = PERCENT_PRECISION) = CompliancePercent.fromLevels(this, precision)

def +(compliance: ComplianceLevel): ComplianceLevel = {
ComplianceLevel(
Expand All @@ -271,6 +271,8 @@ final case class ComplianceLevel(

object ComplianceLevel {

def PERCENT_PRECISION = 2

def compute(reports: Iterable[ReportType]): ComplianceLevel = {
import ReportType._
if(reports.isEmpty) {
Expand Down Expand Up @@ -455,31 +457,30 @@ object ComplianceLevelSerialisation {
(ComplianceLevel.apply _).tupled(parse(json, (i:BigInt) => i.intValue))
}

def parsePercent(json: JValue) = {
(CompliancePercent.apply _).tupled(parse(json, (i:BigInt) => i.doubleValue))
}

//transform the compliance percent to a list with a given order:
// pc_reportDisabled, pc_notapplicable, pc_success, pc_repaired,
// pc_error, pc_pending, pc_noAnswer, pc_missing, pc_unknown
implicit class ComplianceLevelToJs(val compliance: ComplianceLevel) extends AnyVal {

def toJsArray: JsArray = JsArray (
JsArray(compliance.reportsDisabled,JE.Num(compliance.pc.reportsDisabled)) // 0
, JsArray(compliance.notApplicable , JE.Num(compliance.pc.notApplicable)) // 1
, JsArray(compliance.success , JE.Num(compliance.pc.success)) // 2
, JsArray(compliance.repaired , JE.Num(compliance.pc.repaired)) // 3
, JsArray(compliance.error , JE.Num(compliance.pc.error)) // 4
, JsArray(compliance.pending , JE.Num(compliance.pc.pending)) // 5
, JsArray(compliance.noAnswer , JE.Num(compliance.pc.noAnswer)) // 6
, JsArray(compliance.missing , JE.Num(compliance.pc.missing)) // 7
, JsArray(compliance.unexpected , JE.Num(compliance.pc.unexpected)) // 8
, JsArray(compliance.auditNotApplicable , JE.Num(compliance.pc.auditNotApplicable)) // 9
, JsArray(compliance.compliant , JE.Num(compliance.pc.compliant)) // 10
, JsArray(compliance.nonCompliant , JE.Num(compliance.pc.nonCompliant)) // 11
, JsArray(compliance.auditError , JE.Num(compliance.pc.auditError)) // 12
, JsArray(compliance.badPolicyMode , JE.Num(compliance.pc.badPolicyMode)) // 13
)
def toJsArray: JsArray = {
val pc = compliance.computePercent()
JsArray (
JsArray(compliance.reportsDisabled,JE.Num(pc.reportsDisabled)) // 0
, JsArray(compliance.notApplicable , JE.Num(pc.notApplicable)) // 1
, JsArray(compliance.success , JE.Num(pc.success)) // 2
, JsArray(compliance.repaired , JE.Num(pc.repaired)) // 3
, JsArray(compliance.error , JE.Num(pc.error)) // 4
, JsArray(compliance.pending , JE.Num(pc.pending)) // 5
, JsArray(compliance.noAnswer , JE.Num(pc.noAnswer)) // 6
, JsArray(compliance.missing , JE.Num(pc.missing)) // 7
, JsArray(compliance.unexpected , JE.Num(pc.unexpected)) // 8
, JsArray(compliance.auditNotApplicable , JE.Num(pc.auditNotApplicable)) // 9
, JsArray(compliance.compliant , JE.Num(pc.compliant)) // 10
, JsArray(compliance.nonCompliant , JE.Num(pc.nonCompliant)) // 11
, JsArray(compliance.auditError , JE.Num(pc.auditError)) // 12
, JsArray(compliance.badPolicyMode , JE.Num(pc.badPolicyMode)) // 13
)
}

def toJson: JObject = {
import compliance._
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -573,11 +573,11 @@ object NodeStatusReportSerialization {
c match {
case c : ValueStatusReport =>
( ("componentName" -> c.componentName)
~ ("compliance" -> c.compliance.pc.toJson)
~ ("compliance" -> c.compliance.computePercent().toJson)
~ ("numberReports" -> c.compliance.total)
~ ("values" -> c.componentValues.values.map { v =>
( ("value" -> v.componentValue)
~ ("compliance" -> v.compliance.pc.toJson)
~ ("compliance" -> v.compliance.computePercent().toJson)
~ ("numberReports" -> v.compliance.total)
~ ("unexpanded" -> v.unexpandedComponentValue)
~ ("messages" -> v.messages.map { m =>
Expand All @@ -590,7 +590,7 @@ object NodeStatusReportSerialization {
)
case c : BlockStatusReport =>
( ("componentName" -> c.componentName)
~ ("compliance" -> c.compliance.pc.toJson)
~ ("compliance" -> c.compliance.computePercent().toJson)
~ ("numberReports" -> c.compliance.total)
~ ("subComponents" -> c.subComponents.map(componentValueToJson))
~ ("reportingLogic" -> c.reportingLogic.toString)
Expand All @@ -607,11 +607,11 @@ object NodeStatusReportSerialization {

"rules" -> reports.map { r =>
( ("ruleId" -> r.ruleId.serialize)
~ ("compliance" -> r.compliance.pc.toJson)
~ ("compliance" -> r.compliance.computePercent().toJson)
~ ("numberReports" -> r.compliance.total)
~ ("directives" -> r.directives.values.map { d =>
( ("directiveId" -> d.directiveId.serialize)
~ ("compliance" -> d.compliance.pc.toJson)
~ ("compliance" -> d.compliance.computePercent().toJson)
~ ("numberReports" -> d.compliance.total)
~ ("components" -> d.components.values.map(componentValueToJson))
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ final case class RunCompliance(
object RunCompliance {

def from(runTimestamp: DateTime, endOfLife: DateTime, report: NodeStatusReport) = {
RunCompliance(report.nodeId, runTimestamp, endOfLife, (report.runInfo, report.statusInfo), report.compliance.pc, report.reports)
RunCompliance(report.nodeId, runTimestamp, endOfLife, (report.runInfo, report.statusInfo), report.compliance.computePercent(), report.reports)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ trait RuleOrNodeReportingServiceImpl extends ReportingService {

Some((
complianceLevel
, complianceLevel.complianceWithoutPending.round
, complianceLevel.withoutPending.computePercent().compliance.round
))
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,15 +116,15 @@ class StatusReportTest extends Specification {
}

"correctly add them" in {
aggregate(d1c1v1_s ++ d1c2v21_s).compliance.pc.success === 100
aggregate(d1c1v1_s ++ d1c2v21_s).compliance.computePercent().success === 100
}

"correctly add them by directive" in {
val a = aggregate(d1c1v1_s ++ d1c2v21_s ++ d2c2v21_e)

(a.compliance.pc.success === 66.67.toDouble) and
(a.directives("d1").compliance.pc.success === 100) and
(a.directives("d2").compliance.pc.error === 100)
(a.compliance.computePercent().success === 66.67.toDouble) and
(a.directives("d1").compliance.computePercent().success === 100) and
(a.directives("d2").compliance.computePercent().error === 100)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,24 +73,45 @@ class TestComplianceLevel extends Specification {
}


"a compliance must never be rounded below 0.01%" >> {
"a compliance must never be rounded below its precision" >> {

val c = ComplianceLevel(success = 1, error = 100000)
"which is 0.01 by default" >> {
val c = ComplianceLevel(success = 1, error = 100000)
val pc = c.computePercent()
(pc.repaired === 0) and (pc.success === 0.01) and (pc.error === 99.99)
}

"and can be 0" >> {
val c = ComplianceLevel(success = 1, error = 100000)
val pc = CompliancePercent.fromLevels(c, 0)
(pc.repaired === 0) and (pc.success === 1) and (pc.error === 99)
}

(c.pc.repaired === 0) and (c.pc.success === 0.01) and (c.pc.error === 99.99)
"or a lot" >> {
val c = ComplianceLevel(success = 1, error = 1000000000)
val pc = CompliancePercent.fromLevels(c, 5)
(pc.repaired === 0) and (pc.success === 0.00001) and (pc.error === 99.99999)
}

}

"compliance when there is no report is 0" >> {

ComplianceLevel().compliance === 0
ComplianceLevel().computePercent().compliance === 0
}

"Compliance must sum to 100 percent" should {
"Compliance must sum to 100 percent" >> {

val c = ComplianceLevel(0,1,1,1)
"when using default precision" >> {
val c = ComplianceLevel(0,1,1,1)
val pc = c.computePercent()
(pc.success === 33.33) and (pc.repaired === 33.33) and (pc.error === 33.34)
}

(c.pc.success === 33.33) and (c.pc.repaired === 33.33) and (c.pc.error === 33.34)
"when using 0 digits" >> {
val c = ComplianceLevel(0,1,1,1)
val pc = c.computePercent(0)
(pc.success === 33) and (pc.repaired === 33) and (pc.error === 34)
}
}

}