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.

Sign up for GitHub

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

Jump to bottom

sonar-cxx custom metrics are not dispayed/saved #1509

Closed
ivangalkin opened this issue Jun 28, 2018 · 1 comment · Fixed by #1514
Closed

sonar-cxx custom metrics are not dispayed/saved #1509

ivangalkin opened this issue Jun 28, 2018 · 1 comment · Fixed by #1514
Assignees
@ivangalkin
Labels
bug
Milestone
1.2

Comments

@ivangalkin
Copy link
Contributor

  1. go to https://<server>/project/activity?id=<project_id>
  2. see metrics diagram -> choose Custom in the drop-down menu
  3. click Add metric button
  4. select one of the custom sonar-cxx metrics (see CxxMetrics.java) e.g. CppCheck issues
  5. the diagram will show nothing

Plugin version: HEAD
SonarQube version: 7.1.0.11001

I guess, that the CxxLanguage::MetricsCache is not functioning properly. Is it really filled with data? Is it really persisted at the end?
ivangalkin added a commit to ivangalkin/sonar-cxx that referenced this issue Jun 28, 2018
@ivangalkin
deprecate *PUBLIC*API* metrics
923cd00 
* `PUBLIC_API`, `PUBLIC_UNDOCUMENTED_API` and `PUBLIC_DOCUMENTED_API_DENSITY`
  were depricated since SQ 6.2 (https://jira.sonarsource.com/browse/SONAR-8328)
  but existed as custom sonar-cxx metrics

* the minimal supported SQ version now is 6.7

* moreover, it looks like there is a general problem in displaying
  custom metrics (see SonarOpenCommunity#1509)

* that means that nobody a) expects that the SQ plugin implements the
  deprectad metrics and b) nobody misses them (because they are just not
  visualized)

* BTW public API measurements belong to the obligatory AST Visitors
  and introduce the time overhead of ~6% (measured with SonarOpenCommunity#1507,
  6% means, that if there is no sensors activated at all
  the importing of C++ project will still cause a calculation
  of a bunch of metrics; summary overhead of this calculation
  considered as 100%)
@ivangalkin ivangalkin mentioned this issue Jun 28, 2018
Closed
ivangalkin added a commit to ivangalkin/sonar-cxx that referenced this issue Jun 28, 2018
@ivangalkin
deprecate *PUBLIC*API* metrics
ad0da49 
* `PUBLIC_API`, `PUBLIC_UNDOCUMENTED_API` and `PUBLIC_DOCUMENTED_API_DENSITY`
  were depricated since SQ 6.2 (https://jira.sonarsource.com/browse/SONAR-8328)
  but existed as custom sonar-cxx metrics

* the minimal supported SQ version now is 6.7

* moreover, it looks like there is a general problem in displaying
  custom metrics (see SonarOpenCommunity#1509)

* that means that nobody a) expects that the SQ plugin implements the
  deprectad metrics and b) nobody misses them (because they are just not
  visualized)

* the squid check `UndocumentedApiCheck` is not affected
  (it doesn't rely on the stored metric, but visits the AST
  by itself)

* BTW public API measurements belong to the obligatory AST Visitors
  and introduce the time overhead of ~6% (measured with SonarOpenCommunity#1507,
  6% means, that if there is no sensors activated at all
  the importing of C++ project will still cause a calculation
  of a bunch of metrics; summary overhead of this calculation
  considered as 100%)
ivangalkin added a commit to ivangalkin/sonar-cxx that referenced this issue Jun 28, 2018
@ivangalkin
deprecate *PUBLIC*API* metrics
f176ec0 
* `PUBLIC_API`, `PUBLIC_UNDOCUMENTED_API` and `PUBLIC_DOCUMENTED_API_DENSITY`
  were depricated since SQ 6.2 (https://jira.sonarsource.com/browse/SONAR-8328)
  but existed as custom sonar-cxx metrics

* the minimal supported SQ version now is 6.7

* moreover, it looks like there is a general problem in displaying
  custom metrics (see SonarOpenCommunity#1509)

* that means that nobody a) expects that the SQ plugin implements the
  deprectad metrics and b) nobody misses them (because they are just not
  visualized)

* the squid check `UndocumentedApiCheck` is not affected
  (it doesn't rely on the stored metric, but visits the AST
  by itself) but `CxxPublicApiVisitorTest` had to be rewritten

* BTW public API measurements belong to the obligatory AST Visitors
  and introduce the time overhead of ~6% (measured with SonarOpenCommunity#1507,
  6% means, that if there is no sensors activated at all
  the importing of C++ project will still cause a calculation
  of a bunch of metrics; summary overhead of this calculation
  considered as 100%)
@guwirth guwirth added this to the 1.2 milestone Jun 29, 2018
ivangalkin added a commit to ivangalkin/sonar-cxx that referenced this issue Jul 6, 2018
@ivangalkin
work in progress: SonarOpenCommunity#1509
49fc876 
TODO:

* formatting
* renaming of some local vars
* tests for custom metrics?
@ivangalkin
Copy link
Contributor Author

There were multiple causes for the wrong behavior:

  1. the CxxLanguage::MetricsCache didn't function at all: basically it was never filled, the stores and lookups were made with inconsistent keys and results were ignored without any warnings
  2. the PUBLICAPI* metrics were not auto-aggregated, which means that there have to be an additional step in order to accumulate per-file measurement into the per-module metric

The change ivangalkin@49fc876 is pretty large. I will refactor and submit it asap.

@ivangalkin ivangalkin self-assigned this Jul 6, 2018
ivangalkin added a commit to ivangalkin/sonar-cxx that referenced this issue Jul 6, 2018
@ivangalkin
work in progress: SonarOpenCommunity#1509
8f77afc 
TODO:

* formatting
ivangalkin added a commit to ivangalkin/sonar-cxx that referenced this issue Jul 7, 2018
@ivangalkin
Fix SonarOpenCommunity#1509
b18fc80 
Redesign of language-specific metrics
The following problems were identified and solved:

PROBLEM 1
* CxxLanguage::MetricsCache was not filled while
  construction of CxxLanguage object but only in
  CxxSquidSensor::CxxSquidSensor()

SOLUTION
* Introduce CxxMetricsFactory.
* It has no dependencies to sensors.
* It creates language-dependent metrics in the constructor
  of CxxSquidSensor

PROBLEM 2
* During the test MetricsCache was almost always empty.
  In order to prevent warnings (?) missing metrics' lookups
  were just ignored

SOLUTON
* Introduce `protected abstract Optional<CxxMetricsFactory.Key> CxxReportSensor::getMetricKey();`
* By means of Optional<> semantic sensors show if metric is required or not
* CxxLanguage::getMetric() checks thows an exception if metric hasn't been found

PROBLEM 3
* CxxLanguage::getMetric(<required metric key>) expected the key to be typed as String
* In order to fill the MetricsCache the format {METRIC_KEY} was used
* Cache lookups were done however in the format {LANG_PROP_KEY}-{METRIC_KEY}

SOLUTION
* In order to make the lookup key type-safe and less error-prone I introduced the type
  `CxxMetricsFactory.Key`

PROBLEM 4
* The plugin-specific metrics for public API:
  It was not enough to set them on the file level. One had to aggregate these
  measurements and set the result on the module level. Otherwise
  they were not displayed in the SonarQube's Activity UI

SOLUTION
* Introduce `CxxMetricsAggragator` (previously known as `SquidSensor`)
* Minor rework of `CxxSquidSensor`

PROBLEM 5
* The metric `CoreMetrics.COGNITIVE_COMPLEXITY` was measured but not persisted

MISC
* Test has been added
* Minor refactoring and comments
ivangalkin added a commit to ivangalkin/sonar-cxx that referenced this issue Jul 8, 2018
@ivangalkin
Fix SonarOpenCommunity#1509 - apply review comments, add tests
fb74088 
* More metrics-related tests were added to `CxxSquidSensorTest.java`
@ivangalkin ivangalkin mentioned this issue Jul 18, 2018
Merged
ivangalkin added a commit to ivangalkin/sonar-cxx that referenced this issue Jul 19, 2018
@ivangalkin
Fix SonarOpenCommunity#1509
6e08f1c 
Redesign of language-specific metrics
The following problems were identified and solved:

PROBLEM 1
* CxxLanguage::MetricsCache was not filled while
  construction of CxxLanguage object but only in
  CxxSquidSensor::CxxSquidSensor()

SOLUTION
* Introduce CxxMetricsFactory.
* It has no dependencies to sensors.
* It creates language-dependent metrics in the constructor
  of CxxSquidSensor

PROBLEM 2
* During the test MetricsCache was almost always empty.
  In order to prevent warnings (?) missing metrics' lookups
  were just ignored

SOLUTON
* Introduce `protected abstract Optional<CxxMetricsFactory.Key> CxxReportSensor::getMetricKey();`
* By means of Optional<> semantic sensors show if metric is required or not
* CxxLanguage::getMetric() checks thows an exception if metric hasn't been found

PROBLEM 3
* CxxLanguage::getMetric(<required metric key>) expected the key to be typed as String
* In order to fill the MetricsCache the format {METRIC_KEY} was used
* Cache lookups were done however in the format {LANG_PROP_KEY}-{METRIC_KEY}

SOLUTION
* In order to make the lookup key type-safe and less error-prone I introduced the type
  `CxxMetricsFactory.Key`

PROBLEM 4a
* The plugin-specific metrics for public API were missing on module level
  All CoreMetrics (incls. former Public API) have automatic aggregation:
  1. ComputeEngine implements some server-side task
  2. This task iterates over the project hierarchy
     `FILE -> DIRECTORY/MODULE -> ... -> ... -> PROJECT`
     and performs a recursive sum for all metrics raised
     on children component
  3. This sum is stored as aggregated metric for the parent component
  4. The number of hierarchy levels can be arbitrary

  5. Starting with version 6.2 the public API metrics were
     deprecated and Sonar-CXX introduces its own plugin-specific
     public API metrics
  6. These custom metrics are not aggregated automatically
     anymore

PROBLEM 4b
* PR SonarOpenCommunity#1398 introduced 9 new complexity metrics
  They supported the aggregation of the file-level metrics.
  The aggregation was implemeted inside of the `AstVisitor`,
  however the aggregation was done for the parent module only.

  Ths solution worked for the simple SonarQube projects only.
  Due to hierarchy depth == 2, aggregated results were stored
  on the project level. Multi-module projects however might
  have an arbitrary depth. So there was no proper aggregation
  for such projects.

  After public API metrics were deprecated as CoreMetrics
  the automatic aggregation
  It was not enough to set them on the file level. One had to aggregate these
  measurements and set the result on the module level. Otherwise
  they were not displayed in the SonarQube's Activity UI

SOLUTION
* Unify implementation of custom metrics for Public-API and complexity
* Implement 2 Compute Engine tasks (see `MeasureComputer`), which perform
  the aggregation and computation of these metrics
* Computaton/aggregation works for both types of SonarQube project -
  the simple and the multi-module ones

PROBLEM 5
* The metric `CoreMetrics.COGNITIVE_COMPLEXITY` was measured but not persisted

MISC
* Test has been added
* Minor refactoring and comments
guwirth added a commit that referenced this issue Jul 19, 2018
@guwirth
Merge pull request #1514 from ivangalkin/fix_metrics
d4fc380 
Fix #1509 - enable custom metrics introduced by sonar-cxx
@guwirth guwirth mentioned this issue Nov 1, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ivangalkin ivangalkin

Labels
bug
Milestone
1.2
Development

Successfully merging a pull request may close this issue.

Fix #1509 - enable custom metrics introduced by sonar-cxx ivangalkin/sonar-cxx
2 participants
@guwirth @ivangalkin