api,config: enable audit-middleware in default config #5592

CabinfeverB · 2022-10-12T18:10:00Z

Signed-off-by: Cabinfever_B cabinfeveroier@gmail.com

default enable audit

Signed-off-by: Cabinfever_B cabinfeveroier@gmail.com

What problem does this PR solve?

Issue Number: ref #4480

What is changed and how does it work?

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
Benchmark test result
BenchmarkDoRequestWithLocalLogAudit

5206 211411 ns/op 26330 B/op 190 allocs/op
6152 247705 ns/op 26319 B/op 190 allocs/op

BenchmarkDoRequestWithPrometheusAudit

4790 351855 ns/op 31247 B/op 245 allocs/op
4512 276509 ns/op 30732 B/op 240 allocs/op

BenchmarkDoRequestWithoutServiceMiddleware

5767 237742 ns/op 26352 B/op 190 allocs/op
4887 209907 ns/op 26329 B/op 190 allocs/op

Code changes

Has configuration change

Release note

None.

ti-chi-bot · 2022-10-12T18:10:02Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

nolouch
rleungx

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

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

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com> default enable audit Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com> default enable audit Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

codecov · 2022-10-13T05:30:05Z

Codecov Report

Base: 75.74% // Head: 75.67% // Decreases project coverage by -0.06% ⚠️

Coverage data is based on head (5583226) compared to base (bc2019e).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5592      +/-   ##
==========================================
- Coverage   75.74%   75.67%   -0.07%     
==========================================
  Files         326      326              
  Lines       32251    32251              
==========================================
- Hits        24427    24405      -22     
- Misses       5722     5735      +13     
- Partials     2102     2111       +9

Flag	Coverage Δ
unittests	`75.67% <100.00%> (-0.07%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
server/config/service_middleware_config.go	`100.00% <ø> (ø)`
server/metrics.go	`100.00% <ø> (ø)`
pkg/audit/audit.go	`91.66% <100.00%> (ø)`
server/api/router.go	`97.95% <100.00%> (ø)`
pkg/dashboard/keyvisual/input/core.go	`0.00% <0.00%> (-33.34%)`	⬇️
server/storage/endpoint/rule.go	`83.33% <0.00%> (-11.12%)`	⬇️
pkg/tempurl/tempurl.go	`60.00% <0.00%> (-10.00%)`	⬇️
server/tso/local_allocator.go	`64.86% <0.00%> (-6.76%)`	⬇️
server/id/id.go	`86.44% <0.00%> (-3.39%)`	⬇️
server/schedulers/random_merge.go	`62.50% <0.00%> (-3.13%)`	⬇️
... and 22 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

CabinfeverB · 2022-10-13T10:32:31Z

/run-monitor-test

sre-bot · 2022-10-13T10:34:48Z

Visit the grafana server at: http://172.16.5.21:30640, it will last for 5 hours

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com> add grafana Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

nolouch · 2022-10-13T13:07:02Z

/run-monitor-test

sre-bot · 2022-10-13T13:30:31Z

Visit the grafana server at: http://172.16.5.21:32173, it will last for 5 hours

rleungx · 2022-10-14T02:39:41Z

metrics/grafana/pd.json

+          "timeFrom": null,
+          "timeRegions": [],
+          "timeShift": null,
+          "title": "Completed commands rate",


How about adding HTTP prefix？

rleungx · 2022-10-14T02:41:11Z

server/api/router.go


 	rulesHandler := newRulesHandler(svr, rd)
 	registerFunc(clusterRouter, "/config/rules", rulesHandler.GetAllRules, setMethods(http.MethodGet))
-	registerFunc(clusterRouter, "/config/rules", rulesHandler.SetAllRules, setMethods(http.MethodPost), setAuditBackend(localLog))
-	registerFunc(clusterRouter, "/config/rules/batch", rulesHandler.BatchRules, setMethods(http.MethodPost), setAuditBackend(localLog))
+	registerFunc(clusterRouter, "/config/rules", rulesHandler.SetAllRules, setMethods(http.MethodPost), setAuditBackend(localLog), setAuditBackend(prometheus))


Why change it?

add metrics for all API

Suggested change

registerFunc(clusterRouter, "/config/rules", rulesHandler.SetAllRules, setMethods(http.MethodPost), setAuditBackend(localLog), setAuditBackend(prometheus))

registerFunc(clusterRouter, "/config/rules", rulesHandler.SetAllRules, setMethods(http.MethodPost), setAuditBackend(localLog, prometheus))

rleungx · 2022-10-14T02:42:13Z

server/api/router.go

-	registerFunc(apiRouter, "/members/id/{id}", memberHandler.DeleteMemberByID, setMethods(http.MethodDelete), setAuditBackend(localLog))
-	registerFunc(apiRouter, "/members/name/{name}", memberHandler.SetMemberPropertyByName, setMethods(http.MethodPost), setAuditBackend(localLog))
+	registerFunc(apiRouter, "/members", memberHandler.GetMembers, setMethods(http.MethodGet), setAuditBackend(prometheus))
+	registerFunc(apiRouter, "/members/name/{name}", memberHandler.DeleteMemberByName, setMethods(http.MethodDelete), setAuditBackend(localLog), setAuditBackend(prometheus))


rleungx · 2022-10-14T02:43:40Z

BTW, I suggest adding a comment to explain the principle of using logs or metrics.

nolouch · 2022-10-14T03:39:25Z

server/api/router.go

 	registerFunc(apiRouter, "/config/replication-mode", confHandler.GetReplicationModeConfig, setMethods(http.MethodGet))
-	registerFunc(apiRouter, "/config/replication-mode", confHandler.SetReplicationModeConfig, setMethods(http.MethodPost))
+	registerFunc(apiRouter, "/config/replication-mode", confHandler.SetReplicationModeConfig, setMethods(http.MethodPost), setAuditBackend(localLog))


I think also need for Prometheus for this api

rleungx · 2022-10-14T09:11:31Z

metrics/grafana/pd.json

@@ -8176,7 +8176,7 @@
          "timeFrom": null,
          "timeRegions": [],
          "timeShift": null,
-          "title": "Completed commands rate",
+          "title": " gRPC Completed commands rate",


Suggested change

"title": " gRPC Completed commands rate",

"title": "gRPC Completed commands rate",

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com> all api supporting metrics Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com> all api supporting metrics Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

rleungx · 2022-10-17T02:57:56Z

/merge

ti-chi-bot · 2022-10-17T02:57:57Z

@rleungx: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

ti-chi-bot · 2022-10-17T02:57:59Z

This pull request has been accepted and is ready to merge.

Commit hash: fe89dba

ti-chi-bot · 2022-10-17T02:58:12Z

@CabinfeverB: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

ti-chi-bot added the release-note-none Denotes a PR that doesn't merit a release note. label Oct 12, 2022

ti-chi-bot requested review from nolouch and rleungx October 12, 2022 18:10

CabinfeverB changed the title ~~api,config: enable audit in default config~~ api,config: enable audit-middleware in default config Oct 12, 2022

default enable audit

4c9a833

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com> default enable audit Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com> default enable audit Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

CabinfeverB force-pushed the default_enable_audit branch from 6835ab8 to 4c9a833 Compare October 13, 2022 05:22

nolouch approved these changes Oct 13, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Oct 13, 2022

add grafana

fa31d41

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com> add grafana Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

CabinfeverB force-pushed the default_enable_audit branch from e583692 to fa31d41 Compare October 13, 2022 10:59

rleungx reviewed Oct 14, 2022

View reviewed changes

nolouch reviewed Oct 14, 2022

View reviewed changes

rleungx reviewed Oct 14, 2022

View reviewed changes

CabinfeverB force-pushed the default_enable_audit branch from fe42952 to e938484 Compare October 14, 2022 09:19

all api supporting metrics

fe89dba

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com> all api supporting metrics Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com> all api supporting metrics Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

CabinfeverB force-pushed the default_enable_audit branch from e938484 to fe89dba Compare October 14, 2022 09:21

rleungx approved these changes Oct 14, 2022

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Oct 14, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Oct 17, 2022

Merge branch 'master' into default_enable_audit

5583226

ti-chi-bot merged commit 19aa1c8 into tikv:master Oct 17, 2022

CabinfeverB deleted the default_enable_audit branch October 28, 2022 06:21

CabinfeverB mentioned this pull request Nov 2, 2022

More audit logs for PD API #3383

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

api,config: enable audit-middleware in default config #5592

api,config: enable audit-middleware in default config #5592

CabinfeverB commented Oct 12, 2022 •

edited

Loading

ti-chi-bot commented Oct 12, 2022 •

edited

Loading

codecov bot commented Oct 13, 2022 •

edited

Loading

CabinfeverB commented Oct 13, 2022

sre-bot commented Oct 13, 2022

nolouch commented Oct 13, 2022

sre-bot commented Oct 13, 2022

rleungx Oct 14, 2022

rleungx Oct 14, 2022

CabinfeverB Oct 14, 2022

rleungx Oct 14, 2022

rleungx Oct 14, 2022

rleungx commented Oct 14, 2022

nolouch Oct 14, 2022

rleungx Oct 14, 2022

rleungx commented Oct 17, 2022

ti-chi-bot commented Oct 17, 2022

ti-chi-bot commented Oct 17, 2022

ti-chi-bot commented Oct 17, 2022

	registerFunc(clusterRouter, "/config/rules", rulesHandler.SetAllRules, setMethods(http.MethodPost), setAuditBackend(localLog), setAuditBackend(prometheus))
	registerFunc(clusterRouter, "/config/rules", rulesHandler.SetAllRules, setMethods(http.MethodPost), setAuditBackend(localLog, prometheus))

	"title": " gRPC Completed commands rate",
	"title": "gRPC Completed commands rate",

api,config: enable audit-middleware in default config #5592

api,config: enable audit-middleware in default config #5592

Conversation

CabinfeverB commented Oct 12, 2022 • edited Loading

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

ti-chi-bot commented Oct 12, 2022 • edited Loading

codecov bot commented Oct 13, 2022 • edited Loading

Codecov Report

CabinfeverB commented Oct 13, 2022

sre-bot commented Oct 13, 2022

nolouch commented Oct 13, 2022

sre-bot commented Oct 13, 2022

rleungx Oct 14, 2022

Choose a reason for hiding this comment

rleungx Oct 14, 2022

Choose a reason for hiding this comment

CabinfeverB Oct 14, 2022

Choose a reason for hiding this comment

rleungx Oct 14, 2022

Choose a reason for hiding this comment

rleungx Oct 14, 2022

Choose a reason for hiding this comment

rleungx commented Oct 14, 2022

nolouch Oct 14, 2022

Choose a reason for hiding this comment

rleungx Oct 14, 2022

Choose a reason for hiding this comment

rleungx commented Oct 17, 2022

ti-chi-bot commented Oct 17, 2022

ti-chi-bot commented Oct 17, 2022

ti-chi-bot commented Oct 17, 2022

CabinfeverB commented Oct 12, 2022 •

edited

Loading

ti-chi-bot commented Oct 12, 2022 •

edited

Loading

codecov bot commented Oct 13, 2022 •

edited

Loading