[ADR] SonarCloud Code Quality Scanning

Status

  • Proposed
  • Trialing
  • Under review
  • Approved
  • Retired

Context & Scope

The forum has previously decided to use the free version of SonarCloud to scan contributed code for code coverage, bugs, vulnerabilities, maintainability, etc. Until recently, there had been no code scanning used, which resulted in:

  • Lack of visibility of the current code quality and technical debt of each service
  • No mechanisms to control or monitor the code quality of contributions
  • No quantitative quality standard defined by the forum

Code scanning was introduced on all Java projects with this MR: !1130 (merged).

The purpose of this ADR is to define the code quality standard, provide mechanisms for enforcing that standard, and define the scope of work.

Decision

Scanning Common Code Only

The Sonar Scan will be configured to scan only the common code (namely, core and core-plus modules in Java services). Provider code will be excluded. Providers who wish to scan their own code will need to do so separately. The entire project is being scanned temporarily but will be disabled in the future before introducing the requirement to fail the pipeline on a failing quality gate.

Projects

All projects included within the PMC release shall be scanned with Sonar, excluding any CSP-specific projects. GitLab stages will need to be defined for each language and be extensible enough to apply to different project structures.

Quality Gate

See Sonar Metric Definitions

There are two quality gates: New Code (Code Contributed on an MR) and Overall Code. Overall Code is used to define the standard, whereas New Code is useful for monitoring, making progress, and enforcing standards. The New Code should always be a higher standard then the Overall Code, otherwise there will be regressions.

New Code

  1. Blocker Issues = 0
  2. Bugs = 0
  3. Critical Issues = 0
  4. Duplicated Lines <= 10%
  5. Maintainability Rating = A
  6. Major Issues = 0
  7. Test Code Coverage >= 80.0%
  8. Reliability Rating = A
  9. Security Hotspots Reviewed = 100%
  10. Security Rating = A
  11. Vulnerabilities = 0

Overall Code

Same as above except:

  1. Code Smells <= 40
  2. Duplicated Lines <= 30%

Visibility

A link to the Sonar scan result will be added to the MR for visibility.

Enforcement

The quality standard will be enforced by causing pipeline failures in GitLab for all Graduated services as defined by the PMC Capability Maturity Level. Pipeline scanning failures will be caused on all New and Overall Code additions, but not block the completion of the pipeline.

Rationale

Q: Why do we need a code quality standard?

A: Users of the OSDU platform should have trust in the integrity of a graduated service, and comes with an expectation in minimum quality. A high test coverage (80%) is needed to ensure future code changes do not unintentionally break or change the behavior of a service. Similarly, there should not be any known major issues or vulnerabilities.

Q: Why not require code coverage of 100%?

A: Some lines/branches contain no-ops and bring no value to test. In other cases, it can be very difficult to construct a test to get full code coverage. If the target is too high it will become too difficult to meet and sustain adding higher maintenance costs than what would be gained over the long-term. Higher than 80 code coverages are still possible and encouraged but not required so they do not block development.

Q: Why do we allow code smells and higher duplicated lines in Overall Code?

A: From an end-user perspective, code maintainability is less important but should not regress to a point where it becomes too difficult to contribute to the code base, which is why there are looser requirements on maintainability issues (Code Smells <= 40 and Duplicated Lines <= 30%).

Q: Why do we only enforce the quality gate on graduated services?

A: Incubator and Sandbox services are encouraged to follow PMC best practices and meet the code quality standard. High code quality encourages adoption and contributions by the larger community. However, meeting the quality bar while developing a new service may result in extra effort while the business logic and project structure is under constant change. The service may not be adopted either, resulting in wasted effort. Sonar will still scan and report on these projects, so developers will be aware of what issues and test coverage exists.

Q: Why do we only scan common code and not provider implementations?

A: The code quality of the common code will be enforced by the PMC governance. Contributors will be required to contribute code at or above the standard to avoid regressions to the OSDU platform. With the introduction of the Community Implementation, provider implementations will fall outside of the PMC governance. Meeting the code quality standard will be optional for provider implementations and may result in some providers not assigning resources to meet the standard causing Sonar failures in the GitLab pipeline when the common code meets the standard. Similarly, providers who are above the bar will drag up the code coverage metrics so it may seem like quality is better than it actually is.

Q: What if we want to change the quality standard in the future or disable a particular issue from being scanned in Sonar?

A: This will require an additional ADR.

Q: What work will need to be done and by who if this ADR is accepted?

A: An Epic will be created with links to each scanned Graduated project (Incubator and Sandbox projects will be excluded, since they are not required to meet the quality bar) and targeted milestone, similar to the Spring Framework upgrade. The work will be evenly divided by each CSP by each project. Pipeline failures will be enabled once the project meets the quality bar. Definition of done is when all projects meet the quality standard with pipeline failures enabled.

In addition, there will be work to define the GitLab pipelines for all existing projects (i.e. adding definitions for non-Java services, changing scan target to only common code, configuring for different project structures, improving scan visibility, etc). The remaining work will be added to the above Epic and be picked up on a volunteer basis.

Q: Who will be responsible for updating the service when it transitions to a graduated state to meet the quality standard?

The responsibility will fall to the existing project maintainers or any developer(s) from the community. This responsibility may fall to the PMC to request developer cycles for this effort if the project is meeting the rest of the criteria to be a graduated service but still does not meet the quality standard. The expectation is that by defining a standard and making the scan results visible that the project will meet the standard earlier in the project maturity and that no coordinated effort will be required.

Consequences

No project today meets the code quality standard set in this ADR. Meeting the quality bar will require a collective effort to resolve any issues and missing test cases for each project. In addition, enforcing the standard will require the entire community to be aware of the standard and participate in enforcing it.

The benefit is that there will be better awareness of the technical debt, risks, etc and improved integrity of the overall OSDU platform.

When to revisit

Compliance to this ADR should be reviewed regularly (i.e. once per milestone release) until the standard is reached. If no progress is being made then it may be the metrics are overly restrictive.

Tradeoff Analysis - Input to decision

Current list of Graduated services according to the OSDU Platform Standard:

  • Entitlements
  • File
  • Indexer
  • Legal
  • Notification
  • Partition
  • Dataset
  • Policy
  • Register
  • Schema
  • Search
  • Storage
  • CRS Catalog
  • CRS Conversion
  • Unit
  • Seismic DDMS
  • Reservoir DDMS
  • Wellbore DDMS
  • CSV Parser DAG
  • EDS Fetch & Ingest DAG
  • EDS Scheduler DAG
  • External Data Service
  • Manifest-Based Ingestion DAG
  • Segy-to-openVDS Conversion DAG
  • Segy-to-openZGY Conversion DAG
  • Workflow Service
  • Geospatial Consumption Zone (GCZ)

The current status of each project can be found in the OSDU SonarCloud.

Alternatives and implications

The developer version of Sonar includes the addition of MR decorators in GitLab that comes at an additional cost. Using MR decorators will improve the visibility of the scan result. While this option was considered, it is an operational consideration, not a technical one.

Decision criteria and tradeoffs

Decision timeline

Edited by Chad Leong