-
Notifications
You must be signed in to change notification settings - Fork 13
CLOUDP-292850: Investigate diff approach for Metric Collection #346
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file has:
- A new schema defined, named as TestSchema
- Two new paths defined
- OperationId of get method of /api/atlas/v2 path modified
- Schema example of parameter named collectionName is modified
for testing purposes
@yelizhenden-mdb good to close? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a proof of concept for an Incremental Validation Framework that leverages a diff-based approach to merge and lint OpenAPI specifications. Key changes include:
- The addition of a new main.go implementing merging and spectral linting logic.
- New configuration files (.goreleaser.yml and .golangci.yml) established for building and linting the metrics component.
Reviewed Changes
Copilot reviewed 51 out of 68 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
tools/spectral/ipa/metrics/main.go | Implements merging of spec files and spectral linting for tag changes. |
tools/spectral/ipa/metrics/.goreleaser.yml | Provides release configuration, though the main entry point seems misaligned. |
tools/spectral/ipa/metrics/.golangci.yml | Sets up linters; a duplicate linter entry is present for 'whitespace'. |
Files not reviewed (17)
- tools/spectral/ipa/metrics/go.mod: Language not supported
- tools/spectral/ipa/metrics/path_to_key_mapping.json: Language not supported
- tools/spectral/ipa/metrics/split_specs/tags/AWS Clusters DNS/spec.yaml: Language not supported
- tools/spectral/ipa/metrics/split_specs/tags/Access Tracking/spec.yaml: Language not supported
- tools/spectral/ipa/metrics/split_specs/tags/Alert Configurations/spec.yaml: Language not supported
- tools/spectral/ipa/metrics/split_specs/tags/Alerts/spec.yaml: Language not supported
- tools/spectral/ipa/metrics/split_specs/tags/Auditing/spec.yaml: Language not supported
- tools/spectral/ipa/metrics/split_specs/tags/Cloud Migration Service/spec.yaml: Language not supported
- tools/spectral/ipa/metrics/split_specs/tags/Cloud Provider Access/spec.yaml: Language not supported
- tools/spectral/ipa/metrics/split_specs/tags/Cluster Outage Simulation/spec.yaml: Language not supported
- tools/spectral/ipa/metrics/split_specs/tags/Custom Database Roles/spec.yaml: Language not supported
- tools/spectral/ipa/metrics/split_specs/tags/Database Users/spec.yaml: Language not supported
- tools/spectral/ipa/metrics/split_specs/tags/Encryption at Rest using Customer Key Management/spec.yaml: Language not supported
- tools/spectral/ipa/metrics/split_specs/tags/Events/spec.yaml: Language not supported
- tools/spectral/ipa/metrics/split_specs/tags/Flex Clusters/spec.yaml: Language not supported
- tools/spectral/ipa/metrics/split_specs/tags/Flex Restore Jobs/spec.yaml: Language not supported
- tools/spectral/ipa/metrics/split_specs/tags/Flex Snapshots/spec.yaml: Language not supported
env: | ||
- CGO_ENABLED=0 | ||
binary: bin/foascli | ||
main: ./cmd/foascli.go |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The entry point in the goreleaser config points to './cmd/foascli.go', which does not match the new metrics/main.go file. Please update the configuration to reference the correct main file for the metrics component.
main: ./cmd/foascli.go | |
main: metrics/main.go |
Copilot uses AI. Check for mistakes.
- whitespace | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A duplicate 'whitespace' linter entry is present in the configuration. Removing the duplicate will help avoid potential redundancy or confusion.
- whitespace |
Copilot uses AI. Check for mistakes.
Proposed changes
Jira ticket: CLOUDP-292850
PoC for Incremental Validation Framework
This PR introduces a Proof of Concept (PoC) for the Incremental Validation Framework, as outlined in the IPA Validation Framework Technical Design document
Key Findings and Implementation Steps:
Step 1: Splitting the Latest OpenAPI Version
The OpenAPI specification prior to the latest version is split by tags, with a separate file created for shared components.
Primary logic resides in
split_oas.go
.Step 2: Detecting Changes
Changes are identified and classified into three cases: added, deleted, and modified. Primary logic resides in
detect_changes.go
fileComponents Changes:
If any shared components are updated, the shared_components file must also be updated to reflect these changes.
This file is later merged with the tag-split files for affected paths.
Added Paths:
If new paths are identified, tag-split files should be updated accordingly.
If a directory with the corresponding tag name for new path doesn't exist, it will be created. Otherwise, the tag-related file is updated with the new path.
Assumption: All endpoints within a path share the same tag.
Deleted Paths:
Deleted paths are extracted from the split files and placed into a separate
deleted_spec.yaml
file.Spectral validation will not run on split files if they have no additions or updates. Additionally, a tag will not be recorded as
affectedTag
if its only change is the deletion of a path.All
deleted_spec.yaml
files will be merged with theshared_components.yaml
file. Results related to paths (but not components) in the merged file will be flagged asisDeleted=true
.Deleted Components (Not Implemented, as suggestion)
Components removed from the OpenAPI spec are extracted from the
shared_components
file and placed in adeleted_components.yaml
file.Spectral validation will be run on
deleted_components.yaml
file and results will be flagged asisDeleted=true
Modified Paths:
Affected tags are tracked, and only the corresponding split files are updated.
Step 3: Incremental Validation
Only the affected tag-split files are merged with the
shared_components.yaml
file.Spectral linting runs on merged files.
Future optimization: Components could be tailored to include only those referenced in the affected tag-split files.
Primary logic resides in
main.go
.Output Examples:
path_to_key_mapping.json:
Provides a mapping of paths to their corresponding keys, including updates from detected changes.merged_spec.yaml
: Demonstrates a merged specification for Monitoring and Logs tag-related paths along with the shared components file. This merge was necessary because the updated collectionName parameter is referenced in paths associated with the Monitoring and Logs tags. This file represents one of the merged outputs generated after detecting differences between the old and new versions.Notes on Running the Code:
Checklist
Changes to Spectral
Further comments