[Impala-ASF-CR] IMPALA-8583: Add metrics for Basic auth
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13640 ) Change subject: IMPALA-8583: Add metrics for Basic auth .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/13640 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d1b1bcc093e4b00802bc108ff4d8d4e2cdfd88f Gerrit-Change-Number: 13640 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 14 Jun 2019 23:06:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8583: Add metrics for Basic auth
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13640 ) Change subject: IMPALA-8583: Add metrics for Basic auth .. IMPALA-8583: Add metrics for Basic auth The patch adds two metrics: impala.thrift-server.hiveserver2-http-frontend.total-basic-auth-success impala.thrift-server.hiveserver2-http-frontend.total-basic-auth-failure which track the number of successful and failed attempts at using Basic auth to authentication to the hiveserver2 http server. A followup patch will add similar metrics for SASL attempts on the beeswax and hiveserver2 binary servers. Testing: - Modified existing HTTP auth test to check that the produced metrics are correct. Change-Id: I6d1b1bcc093e4b00802bc108ff4d8d4e2cdfd88f Reviewed-on: http://gerrit.cloudera.org:8080/13640 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/rpc/auth-provider.h M be/src/rpc/authentication.cc M be/src/rpc/thrift-server.cc M be/src/rpc/thrift-server.h M be/src/transport/THttpServer.cpp M be/src/transport/THttpServer.h M common/thrift/metrics.json M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java 8 files changed, 111 insertions(+), 22 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/13640 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I6d1b1bcc093e4b00802bc108ff4d8d4e2cdfd88f Gerrit-Change-Number: 13640 Gerrit-PatchSet: 5 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-8583: Add metrics for Basic auth
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13640 ) Change subject: IMPALA-8583: Add metrics for Basic auth .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4477/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/13640 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d1b1bcc093e4b00802bc108ff4d8d4e2cdfd88f Gerrit-Change-Number: 13640 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 14 Jun 2019 17:35:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8583: Add metrics for Basic auth
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13640 ) Change subject: IMPALA-8583: Add metrics for Basic auth .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13640 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d1b1bcc093e4b00802bc108ff4d8d4e2cdfd88f Gerrit-Change-Number: 13640 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 14 Jun 2019 17:35:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8583: Add metrics for Basic auth
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13640 ) Change subject: IMPALA-8583: Add metrics for Basic auth .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13640 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d1b1bcc093e4b00802bc108ff4d8d4e2cdfd88f Gerrit-Change-Number: 13640 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 14 Jun 2019 00:19:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8583: Add metrics for Basic auth
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13640 ) Change subject: IMPALA-8583: Add metrics for Basic auth .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3620/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/13640 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d1b1bcc093e4b00802bc108ff4d8d4e2cdfd88f Gerrit-Change-Number: 13640 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 14 Jun 2019 00:13:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8583: Add metrics for Basic auth
Hello Todd Lipcon, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13640 to look at the new patch set (#3). Change subject: IMPALA-8583: Add metrics for Basic auth .. IMPALA-8583: Add metrics for Basic auth The patch adds two metrics: impala.thrift-server.hiveserver2-http-frontend.total-basic-auth-success impala.thrift-server.hiveserver2-http-frontend.total-basic-auth-failure which track the number of successful and failed attempts at using Basic auth to authentication to the hiveserver2 http server. A followup patch will add similar metrics for SASL attempts on the beeswax and hiveserver2 binary servers. Testing: - Modified existing HTTP auth test to check that the produced metrics are correct. Change-Id: I6d1b1bcc093e4b00802bc108ff4d8d4e2cdfd88f --- M be/src/rpc/auth-provider.h M be/src/rpc/authentication.cc M be/src/rpc/thrift-server.cc M be/src/rpc/thrift-server.h M be/src/transport/THttpServer.cpp M be/src/transport/THttpServer.h M common/thrift/metrics.json M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java 8 files changed, 111 insertions(+), 22 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/13640/3 -- To view, visit http://gerrit.cloudera.org:8080/13640 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6d1b1bcc093e4b00802bc108ff4d8d4e2cdfd88f Gerrit-Change-Number: 13640 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-8583: Add metrics for BASIC auth
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/13640 ) Change subject: IMPALA-8583: Add metrics for BASIC auth .. Patch Set 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/13640/1/be/src/rpc/auth-provider.h File be/src/rpc/auth-provider.h: http://gerrit.cloudera.org:8080/#/c/13640/1/be/src/rpc/auth-provider.h@33 PS1, Line 33: AuthProvid > this doesn't seem right Done http://gerrit.cloudera.org:8080/#/c/13640/1/be/src/transport/THttpServer.h File be/src/transport/THttpServer.h: http://gerrit.cloudera.org:8080/#/c/13640/1/be/src/transport/THttpServer.h@84 PS1, Line 84: THttpSer > no need for explicit on multi-arg ctor Done http://gerrit.cloudera.org:8080/#/c/13640/1/be/src/transport/THttpServer.cpp File be/src/transport/THttpServer.cpp: http://gerrit.cloudera.org:8080/#/c/13640/1/be/src/transport/THttpServer.cpp@152 PS1, Line 152: total_basic_auth_success_->Increment(1); > semi-related to this patch: do we do any logging of the incorrect login att Yes, we log a warning with the username and ldap error on all failed attempts, as well as having a variety of other logging around different sasl/kerberos errors, and some vlog-ing on successful attempts. http://gerrit.cloudera.org:8080/#/c/13640/1/common/thrift/metrics.json File common/thrift/metrics.json: http://gerrit.cloudera.org:8080/#/c/13640/1/common/thrift/metrics.json@945 PS1, Line 945: Basic (use > in the long description perhaps we should say Basic (username/password) Aut Done http://gerrit.cloudera.org:8080/#/c/13640/1/common/thrift/metrics.json@949 PS1, Line 949: Basic > nit: BASIC isn't an acronym so I think it should just be capitalized as "Ba Done http://gerrit.cloudera.org:8080/#/c/13640/1/common/thrift/metrics.json@955 PS1, Line 955: auth > similar to above, maybe say something like "due to an invalid username and Done -- To view, visit http://gerrit.cloudera.org:8080/13640 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d1b1bcc093e4b00802bc108ff4d8d4e2cdfd88f Gerrit-Change-Number: 13640 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 13 Jun 2019 23:24:42 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8583: Add metrics for BASIC auth
Hello Todd Lipcon, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13640 to look at the new patch set (#2). Change subject: IMPALA-8583: Add metrics for BASIC auth .. IMPALA-8583: Add metrics for BASIC auth The patch adds two metrics: impala.thrift-server.hiveserver2-http-frontend.total-basic-auth-success impala.thrift-server.hiveserver2-http-frontend.total-basic-auth-failure which track the number of successful and failed attempts at using BASIC auth to authentication to the hiveserver2 http server. A followup patch will add similar metrics for SASL attempts on the beeswax and hiveserver2 binary servers. Testing: - Modified existing HTTP auth test to check that the produced metrics are correct. Change-Id: I6d1b1bcc093e4b00802bc108ff4d8d4e2cdfd88f --- M be/src/rpc/auth-provider.h M be/src/rpc/authentication.cc M be/src/rpc/thrift-server.cc M be/src/rpc/thrift-server.h M be/src/transport/THttpServer.cpp M be/src/transport/THttpServer.h M common/thrift/metrics.json M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java 8 files changed, 111 insertions(+), 22 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/13640/2 -- To view, visit http://gerrit.cloudera.org:8080/13640 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6d1b1bcc093e4b00802bc108ff4d8d4e2cdfd88f Gerrit-Change-Number: 13640 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-8583: Add metrics for BASIC auth
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13640 ) Change subject: IMPALA-8583: Add metrics for BASIC auth .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3614/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/13640 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d1b1bcc093e4b00802bc108ff4d8d4e2cdfd88f Gerrit-Change-Number: 13640 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 13 Jun 2019 20:11:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8583: Add metrics for BASIC auth
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13640 ) Change subject: IMPALA-8583: Add metrics for BASIC auth .. Patch Set 1: (6 comments) http://gerrit.cloudera.org:8080/#/c/13640/1/be/src/rpc/auth-provider.h File be/src/rpc/auth-provider.h: http://gerrit.cloudera.org:8080/#/c/13640/1/be/src/rpc/auth-provider.h@33 PS1, Line 33: MatricGroup this doesn't seem right http://gerrit.cloudera.org:8080/#/c/13640/1/be/src/transport/THttpServer.h File be/src/transport/THttpServer.h: http://gerrit.cloudera.org:8080/#/c/13640/1/be/src/transport/THttpServer.h@84 PS1, Line 84: explicit no need for explicit on multi-arg ctor http://gerrit.cloudera.org:8080/#/c/13640/1/be/src/transport/THttpServer.cpp File be/src/transport/THttpServer.cpp: http://gerrit.cloudera.org:8080/#/c/13640/1/be/src/transport/THttpServer.cpp@152 PS1, Line 152: total_basic_auth_success_->Increment(1); semi-related to this patch: do we do any logging of the incorrect login attempts? http://gerrit.cloudera.org:8080/#/c/13640/1/common/thrift/metrics.json File common/thrift/metrics.json: http://gerrit.cloudera.org:8080/#/c/13640/1/common/thrift/metrics.json@945 PS1, Line 945: BASIC auth in the long description perhaps we should say Basic (username/password) Auth, or something? People may not understand the underlying HTTP mechanism but would like to understand that this represents incorrect password attempts http://gerrit.cloudera.org:8080/#/c/13640/1/common/thrift/metrics.json@949 PS1, Line 949: BASIC nit: BASIC isn't an acronym so I think it should just be capitalized as "Basic" http://gerrit.cloudera.org:8080/#/c/13640/1/common/thrift/metrics.json@955 PS1, Line 955: auth similar to above, maybe say something like "due to an invalid username and password" -- To view, visit http://gerrit.cloudera.org:8080/13640 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d1b1bcc093e4b00802bc108ff4d8d4e2cdfd88f Gerrit-Change-Number: 13640 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 13 Jun 2019 19:45:38 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8583: Add metrics for BASIC auth
Thomas Marshall has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13640 Change subject: IMPALA-8583: Add metrics for BASIC auth .. IMPALA-8583: Add metrics for BASIC auth The patch adds two metrics: impala.thrift-server.hiveserver2-http-frontend.total-basic-auth-success impala.thrift-server.hiveserver2-http-frontend.total-basic-auth-failure which track the number of successful and failed attempts at using BASIC auth to authentication to the hiveserver2 http server. A followup patch will add similar metrics for SASL attempts on the beeswax and hiveserver2 binary servers. Testing: - Modified existing HTTP auth test to check that the produced metrics are correct. Change-Id: I6d1b1bcc093e4b00802bc108ff4d8d4e2cdfd88f --- M be/src/rpc/auth-provider.h M be/src/rpc/authentication.cc M be/src/rpc/thrift-server.cc M be/src/rpc/thrift-server.h M be/src/transport/THttpServer.cpp M be/src/transport/THttpServer.h M common/thrift/metrics.json M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java 8 files changed, 113 insertions(+), 22 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/13640/1 -- To view, visit http://gerrit.cloudera.org:8080/13640 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I6d1b1bcc093e4b00802bc108ff4d8d4e2cdfd88f Gerrit-Change-Number: 13640 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall
