Skip to content

add galley config validation metrics#6762

Closed
ayj wants to merge 6 commits intoistio:masterfrom
ayj:add-galley-validation-telemetry
Closed

add galley config validation metrics#6762
ayj wants to merge 6 commits intoistio:masterfrom
ayj:add-galley-validation-telemetry

Conversation

@ayj
Copy link
Copy Markdown
Contributor

@ayj ayj commented Jun 30, 2018

No description provided.

@ayj ayj requested review from douglas-reid and ozevren June 30, 2018 15:40
ozevren
ozevren approved these changes Jun 30, 2018
View reviewed changes
galley/pkg/crd/validation/webhook.go Outdated
"istio.io/istio/pkg/log"
)

var (
Copy link
Copy Markdown
Contributor

@ozevren ozevren Jun 30, 2018

Choose a reason for hiding this comment

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

Minor nit: Could you put these in a "monitoring.go" file, to keep things organized? We've done this in Mixer:
https://github.com/istio/istio/blob/master/mixer/pkg/runtime/config/monitoring.go

@ozevren
Copy link
Copy Markdown
Contributor

ozevren commented Jun 30, 2018

/lgtm
/approve

@codecov
Copy link
Copy Markdown

codecov bot commented Jun 30, 2018 •
edited
Loading

Codecov Report

Merging #6762 into master will increase coverage by 1%.
The diff coverage is 85%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #6762    +/-   ##
=======================================
+ Coverage      71%     71%    +1%     
=======================================
  Files         368     367     -1     
  Lines       31874   31276   -598     
=======================================
- Hits        22455   22111   -344     
+ Misses       8490    8278   -212     
+ Partials      929     887    -42
Impacted Files Coverage Δ
galley/pkg/crd/validation/monitoring.go 100% <100%> (ø)
galley/pkg/crd/validation/webhook.go 80% <65%> (-1%) ⬇️
pilot/pkg/networking/core/v1alpha3/configgen.go 0% <0%> (-100%) ⬇️
pilot/pkg/networking/core/v1alpha3/cluster.go 0% <0%> (-38%) ⬇️
pilot/pkg/config/memory/monitor.go 82% <0%> (-9%) ⬇️
...olarwinds/internal/papertrail/papertrail_logger.go 59% <0%> (-3%) ⬇️
mixer/adapter/signalfx/metrics.go 41% <0%> (-2%) ⬇️
mixer/adapter/statsd/statsd.go 96% <0%> (-1%) ⬇️
mixer/adapter/kubernetesenv/kubernetesenv.go 84% <0%> (ø) ⬇️
pilot/pkg/model/conversion.go 89% <0%> (ø) ⬇️
... and 24 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a82c55a...01c8bc3. Read the comment docs.

@ozevren
Copy link
Copy Markdown
Contributor

ozevren commented Jun 30, 2018

/lgtm
/approve

@istio-testing
Copy link
Copy Markdown
Collaborator

istio-testing commented Jun 30, 2018

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ayj, ozevren
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: douglas-reid

Assign the PR to them by writing /assign @douglas-reid in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@istio-testing
Copy link
Copy Markdown
Collaborator

istio-testing commented Jun 30, 2018

New changes are detected. LGTM label has been removed.

This was referenced Jul 3, 2018
Closed
Merged
@ayj
Copy link
Copy Markdown
Contributor Author

ayj commented Jul 6, 2018

Dup of #6827 (1.0 release branch)

@ayj ayj closed this Jul 6, 2018
@ayj ayj deleted the add-galley-validation-telemetry branch October 17, 2018 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@douglas-reid douglas-reid Awaiting requested review from douglas-reid

1 more reviewer

@ozevren ozevren ozevren approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@ozevren ozevren

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ayj @ozevren @istio-testing @googlebot