[ https://issues.apache.org/jira/browse/HIVE-26670?focusedWorklogId=822048&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-822048 ]
ASF GitHub Bot logged work on HIVE-26670: ----------------------------------------- Author: ASF GitHub Bot Created on: 31/Oct/22 18:06 Start Date: 31/Oct/22 18:06 Worklog Time Spent: 10m Work Description: ayushtkn commented on code in PR #3710: URL: https://github.com/apache/hive/pull/3710#discussion_r1009703349 ########## jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java: ########## @@ -813,6 +817,27 @@ RegistryBuilder.<ConnectionSocketFactory> create().register("https", socketFacto return httpClientBuilder.build(); } + private boolean isRequestTrackingEnabled() { + return Boolean.valueOf(sessConfMap.get(JdbcConnectionParams.JDBC_PARAM_REQUEST_TRACK)); Review Comment: Can replace with: ``` return Boolean.parseBoolean(sessConfMap.get(JdbcConnectionParams.JDBC_PARAM_REQUEST_TRACK)); ``` Boolean.valueOf does a parseBoolean which returns a primitive boolean, which is converted into Boolean and now you are again returning a primitive boolean, the conversion from boolean to Boolean is unnecessary ########## jdbc/src/java/org/apache/hive/jdbc/HttpDefaultResponseInterceptor.java: ########## @@ -0,0 +1,40 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hive.jdbc; + +import java.io.IOException; + +import org.apache.hadoop.hive.conf.Constants; +import org.apache.http.HttpException; +import org.apache.http.HttpResponse; +import org.apache.http.protocol.HttpContext; + +public class HttpDefaultResponseInterceptor extends HttpResponseInterceptorBase { + + @Override + public void process(HttpResponse response, HttpContext context) throws HttpException, IOException { + String trackHeader = (String) context.getAttribute(Constants.HTTP_HEADER_REQUEST_TRACK); + if (trackHeader == null) { + return; + } + long elapsed = System.currentTimeMillis() - (long) context.getAttribute(trackHeader + "_TIME"); Review Comment: nit: Can use ``Constants.TIME_POSTFIX_REQUEST_TRACK`` instead of hardcoding ``_TIME`` here ########## jdbc/src/java/org/apache/hive/jdbc/HttpRequestInterceptorBase.java: ########## @@ -77,6 +88,14 @@ public void process(HttpRequest httpRequest, HttpContext httpContext) if (isCookieEnabled) { httpContext.setAttribute(Utils.HIVE_SERVER2_RETRY_KEY, Utils.HIVE_SERVER2_CONST_FALSE); } + + if (requestTrackingEnabled) { + String trackHeader = getNewTrackHeader(); + LOG.info("{}:{}", Constants.HTTP_HEADER_REQUEST_TRACK, trackHeader); + additionalHeaders.put(Constants.HTTP_HEADER_REQUEST_TRACK, trackHeader); + httpContext.setAttribute(Constants.HTTP_HEADER_REQUEST_TRACK, trackHeader); + httpContext.setAttribute(trackHeader + Constants.TIME_POSTFIX_REQUEST_TRACK, System.currentTimeMillis()); Review Comment: We are using ``System.currentTimeMillis()`` to calculate the duration. I am not very sure, if using Time.monotonicNow() Will be better. Just by the doc: https://github.com/apache/hadoop/blob/b1f418f8027c2d2a1a1c17ec3a8d23ceaaf8d89c/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Time.java#L49-L69 Issue Time Tracking ------------------- Worklog Id: (was: 822048) Time Spent: 3h 40m (was: 3.5h) > Track every single HTTP request between beeline and hs2 > ------------------------------------------------------- > > Key: HIVE-26670 > URL: https://issues.apache.org/jira/browse/HIVE-26670 > Project: Hive > Issue Type: Improvement > Reporter: László Bodor > Assignee: László Bodor > Priority: Major > Labels: pull-request-available > Time Spent: 3h 40m > Remaining Estimate: 0h > > In my use-case there is a couple of layers between beeline and hs2 (load > balancer, proxies), and I'm about to add something to the requests > (optionally), that can be simply written to access logs in as many layers as > possible. > So instead of: > {code} > 2022-10-18T04:34:25+00:00 - [10.80.143.240, 10.80.143.240] - [hs2 host] - [-] > "POST /cliservice HTTP/1.1" 200 1069 "-" "Java/THttpClient/HC" 703 0.008... > {code} > I want to see something specific to the request, which is generated on client > side, so client can also log it. > As the title suggests, I'm targeting http transport. -- This message was sent by Atlassian Jira (v8.20.10#820010)