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

feat: add overview page + push metrics #3292

Merged
merged 11 commits into from
Sep 7, 2018
Merged

feat: add overview page + push metrics #3292

merged 11 commits into from
Sep 7, 2018

Conversation

sguiheux
Copy link
Collaborator

@sguiheux sguiheux commented Sep 5, 2018

  1. Description
  2. Related issues
  3. About tests
  4. Mentions

@ovh/cds

golangcibot
golangcibot reviewed Sep 5, 2018
View reviewed changes
engine/elasticsearch/elasticsearch_handlers.go Outdated
stringQuery = fmt.Sprintf("%s AND application_id:%d", stringQuery, request.ApplicationID)
}
if request.WorkflowID != 0 {
stringQuery = fmt.Sprintf("%s AND workflow_id:%s", stringQuery, request.WorkflowID)
Copy link

Choose a reason for hiding this comment

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

Sprintf format %s has arg request.WorkflowID of wrong type int64

fsamin
fsamin requested changes Sep 5, 2018
View reviewed changes
engine/api/application_metric.go
"github.com/ovh/cds/sdk"
)

func (api *API) getApplicationMetricHandler() service.Handler {
Copy link
Member

Choose a reason for hiding this comment

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

getApplicationOverviewHandler ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Let this one for now

engine/api/metrics/elasticsearch.go Outdated

var metricsChan chan sdk.Metric

func Init(c context.Context, db gorp.SqlExecutor) {
Copy link
Member

Choose a reason for hiding this comment

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

you should take a dbFunc as an argument

engine/api/workflow/dao_node_run_vulnerability.go Outdated
if errS != nil {
log.Error("HandleVulnerabilityReport> Unable to get summary to create metrics: %s", err)
}
if vulnsDBSummary != nil && errS != nil {
Copy link
Member

Choose a reason for hiding this comment

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

errS == nil ?

sdk/metric.go Outdated
)

// Metric represent a CDS metric
type Metric struct {
Copy link
Member

Choose a reason for hiding this comment

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

What about a Overview type ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

same as above

ui/src/app/model/application.model.ts Outdated
Severity.DEFCON1
];

static getMetricsColors(s: string) {
Copy link
Member

Choose a reason for hiding this comment

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

getSeveritiesColors ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

getColors will be enough

bnjjj
bnjjj suggested changes Sep 5, 2018
View reviewed changes
engine/api/application/application_vunerability.go Outdated
}

if result.Valid {
if err := json.Unmarshal([]byte(result.String), &summary); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

gorpmapping.JSONNullString

engine/api/metrics/elasticsearch.go Outdated
case e := <-metricsChan:
esServices, errS := services.FindByType(db, services.TypeElasticsearch)
if errS != nil {
log.Error("metrics.send> Unable to get elasticsearch service: %v", errS)
Copy link
Contributor

Choose a reason for hiding this comment

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

s/metrics.send/pushInElasticSearch

engine/api/metrics/elasticsearch.go Outdated

code, errD := services.DoJSONRequest(context.Background(), esServices, "POST", "/metrics", e, nil)
if code >= 400 || errD != nil {
log.Error("metrics.send> Unable to send metrics to elasticsearch [%d]: %v", code, errD)
Copy link
Contributor

Choose a reason for hiding this comment

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

s/metrics.send/pushInElasticSearch

ui/src/app/views/application/show/home/application.home.component.ts Outdated

ngOnInit(): void {
this.dashboards = new Array<GraphConfiguration>();
this._appNoCache.getMetrics(this.project.key, this.application.name, 'Vulnerability').subscribe(d => {
Copy link
Contributor

Choose a reason for hiding this comment

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

loading ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

ui/src/app/views/application/show/home/application.home.component.ts Outdated
let found = false;
metrics.forEach(m => {
if (m.value[s]) {
found = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

use .some or for ... with a break

@ovh-cds
Copy link
Collaborator

ovh-cds commented Sep 5, 2018

CDS Report build-ui#5806.0 ✘

  • Compile
    • Build UI ✔
    • TU All -
    • TU model ✘
    • TU service ✘
    • TU shared ✘
    • TU views ✘

@ovh-cds
Copy link
Collaborator

ovh-cds commented Sep 5, 2018

CDS Report build-engine-cli-tools#5807.0 ✘

  • Build minimal
    • cds cli legacy ✔
    • cdsctl linux ✔
    • Engine Build linux amd64 ✔
    • tools/debpacker ✘
    • tools/os-ansible-inventory ✔
    • Worker linux amd64 / 386 ✔

@ovh-cds
Copy link
Collaborator

ovh-cds commented Sep 5, 2018

CDS Report build-ui#5807.0 ✘

  • Compile
    • Build UI ✔
    • TU All -
    • TU model ✘
    • TU service ✘
    • TU shared ✘
    • TU views ✘

@ovh-cds
Copy link
Collaborator

ovh-cds commented Sep 7, 2018

CDS Report build-ui#5824.0 ✘

  • Compile
    • Build UI ✔
    • TU All -
    • TU model ✘
    • TU service ✘
    • TU shared ✘
    • TU views ✘

richardlt
richardlt reviewed Sep 7, 2018
View reviewed changes
engine/api/ui.go Show resolved Hide resolved
engine/api/metrics/elasticsearch.go Show resolved Hide resolved
bnjjj
bnjjj suggested changes Sep 7, 2018
View reviewed changes
ui/src/assets/i18n/fr.json Outdated
"application_home_repo": "Dépôt GIT",
"application_home_no_repo": "Lier un dépôt GIT",
"application_home_platform": "Liste des platformes liées",
"application_home_no_platform": "Lier une platforme",
Copy link
Contributor

Choose a reason for hiding this comment

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

s/platforme/plateforme

@ovh-cds
Copy link
Collaborator

ovh-cds commented Sep 7, 2018

CDS Report build-ui#5830.0 ✘

  • Compile

    • Build UI ✔
    • TU All -
    • TU model ✔
    • TU service ✘
    • TU shared ✔
    • TU views ✔
      Unit Tests Report

  • HeadlessChrome 0.0.0 (Linux 0.0.0)

  • HeadlessChrome 0.0.0 (Linux 0.0.0).1935902

    • CDS: project Store should update Project ✘

  • HeadlessChrome 0.0.0 (Linux 0.0.0).1935903

  • HeadlessChrome 0.0.0 (Linux 0.0.0).1935904

@bnjjj bnjjj merged commit cdf70e0 into master Sep 7, 2018
@sguiheux sguiheux deleted the overview branch September 20, 2018 14:41
This was referenced Feb 20, 2022
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@golangcibot golangcibot golangcibot left review comments

@richardlt richardlt richardlt approved these changes

@bnjjj bnjjj bnjjj approved these changes

@fsamin fsamin fsamin approved these changes

@yesnault yesnault yesnault approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@sguiheux @ovh-cds @yesnault @fsamin @richardlt @bnjjj @golangcibot