Skip to content

Commit

Permalink
Refactor Http Client implementation (#1741)
Browse files Browse the repository at this point in the history
Refactor the implementation using java.net with OkHttp.

The current HTTP client has various issues. Instead of pathing the existing implementations, it's better to use another client library. OkHTTPClient is one of the most popular one and it's used by PinDeploy and Spinnaker. 

## Improvements
1. Observability for outgoing requests.
2. 1 shared connection pool managed by OkHttp instead of many ad-hoc connections.
3. Exponential backup retry on certain failures.
4. Logging with configurable level and sensitive header redaction.
4. Moved to universal so Rodimus can use the same implementation

## Tests and validations
Thanks to the added observability, it's very easy to know and monitor if this refactoring is working in addition to the unit tests.

### Smoke tests
Deploy both service and worker to dev1 and ensure these are working
- Github integration - commit data fetch
- Rodimus integration -  Ec2 tags can load
- Changefeed - events are published to changefeed
- Webhook - webhooks can be invoked

## Additional details
### Various logging levels
#### Basic
<img width="980" alt="Screenshot 2024-11-07 at 15 34 48" src="https://github.com/user-attachments/assets/5ac7359d-3011-413d-b5cf-1e6d8c11b346">

#### Body and header
<img width="146" alt="Screenshot 2024-11-07 at 15 35 46" src="https://github.com/user-attachments/assets/85b65d95-7d61-47f3-a62a-d8a4d4fd197b">

### Metrics
<img width="1628" alt="Screenshot 2024-11-07 at 15 31 55" src="https://github.com/user-attachments/assets/8a414b08-f03e-47a4-a70c-2a2e64e7c533">
<img width="1589" alt="Screenshot 2024-11-07 at 15 31 42" src="https://github.com/user-attachments/assets/45dffe1d-329c-4460-8ad6-57599db8055f">
  • Loading branch information
tylerwowen authored Nov 13, 2024
1 parent f7b98a5 commit df4cc0d
Show file tree
Hide file tree
Showing 18 changed files with 810 additions and 877 deletions.
6 changes: 6 additions & 0 deletions deploy-service/common/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -207,5 +207,11 @@
<groupId>org.quartz-scheduler</groupId>
<artifactId>quartz</artifactId>
</dependency>
<dependency>
<groupId>com.squareup.okhttp3</groupId>
<artifactId>mockwebserver</artifactId>
<version>${okhttp.version}</version>
<scope>test</scope>
</dependency>
</dependencies>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
*/
package com.pinterest.deployservice.common;

import com.pinterest.teletraan.universal.http.HttpClient;
import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
Expand All @@ -30,19 +30,17 @@

public final class ChangeFeedJob implements Callable<Void> {
private static final Logger LOG = LoggerFactory.getLogger(ChangeFeedJob.class);
private final int RETRIES = 3;
private static final HttpClient httpClient = HttpClient.builder().build();
private String payload;
private String changeFeedUrl;
private Object oriObj;
private Object curObj;
private HTTPClient httpClient;

public ChangeFeedJob(String payload, String changeFeedUrl, Object oriObj, Object curObj) {
this.payload = payload;
this.changeFeedUrl = changeFeedUrl;
this.oriObj = oriObj;
this.curObj = curObj;
this.httpClient = new HTTPClient();
}

private static String toStringRepresentation(Object object) throws IllegalAccessException {
Expand Down Expand Up @@ -184,9 +182,7 @@ public Void call() {

if (!StringUtils.isEmpty(changeFeedUrl)) {
LOG.info(String.format("Send change feed %s to %s", payload, changeFeedUrl));
Map<String, String> headers = new HashMap<>();
headers.put("Content-Type", "application/json");
httpClient.post(changeFeedUrl, payload, headers, RETRIES);
httpClient.post(changeFeedUrl, payload, null);
}
} catch (Throwable t) {
LOG.error(String.format("Failed to send change feed: %s", payload), t);
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import com.google.gson.JsonObject;
import com.google.gson.JsonParser;
import com.pinterest.teletraan.universal.http.HttpClient;
import java.util.HashMap;
import java.util.Map;
import org.slf4j.Logger;
Expand All @@ -26,8 +27,7 @@
/** Wrapper for Jenkins API calls */
public class Jenkins {
private static final Logger LOG = LoggerFactory.getLogger(Jenkins.class);
private static final int RETRIES = 3;
private HTTPClient httpClient = new HTTPClient();
private static final HttpClient httpClient = HttpClient.builder().build();
private String jenkinsUrl;
private String jenkinsRemoteToken;

Expand Down Expand Up @@ -87,9 +87,8 @@ public boolean isPinterestJenkinsUrl(String url) {

String getJenkinsToken() throws Exception {
String url = String.format("%s/%s", this.jenkinsUrl, "crumbIssuer/api/json");
String ret = httpClient.get(url, null, null, null, RETRIES);
JsonParser parser = new JsonParser();
JsonObject json = (JsonObject) parser.parse(ret);
String ret = httpClient.get(url, null, null);
JsonObject json = (JsonObject) JsonParser.parseString(ret);
return json.get("crumb").getAsString();
}

Expand All @@ -98,7 +97,7 @@ public void startBuild(String url) throws Exception {
Map<String, String> headers = new HashMap<>(1);
headers.put(".crumb", token);
LOG.debug("Calling jenkins with url " + url + " and token " + token);
httpClient.post(url, null, headers, RETRIES);
httpClient.post(url, null, headers);
}

public void startBuild(String jobName, String buildParams) throws Exception {
Expand All @@ -110,18 +109,16 @@ public void startBuild(String jobName, String buildParams) throws Exception {
String.format(
"%s/job/%s/buildWithParameters?%s%s",
this.jenkinsUrl, jobName, tokenString, buildParams);
// startBuild(url);
// Use GET instead, which is the same as POST but no need for token
httpClient.get(url, null, null, null, RETRIES);
httpClient.get(url, null, null);
LOG.info("Successfully post to jenkins for job " + jobName);
}

public Build getBuild(String jobName, String jobNum) throws Exception {
String url = String.format("%s/job/%s/%s/api/json", this.jenkinsUrl, jobName, jobNum);
LOG.debug("Calling jenkins with url " + url);
String ret = httpClient.get(url, null, null, null, RETRIES);
JsonParser parser = new JsonParser();
JsonObject json = (JsonObject) parser.parse(ret);
String ret = httpClient.get(url, null, null);
JsonObject json = (JsonObject) JsonParser.parseString(ret);
return new Build(
json.get("number").toString(),
json.get("result").toString(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -363,8 +363,7 @@ void transition(DeployBean deployBean, DeployBean newDeployBean, EnvironBean env
dataHandler.getDataById(
envBean.getWebhooks_config_id(), WebhookDataFactory.class);
if (webhooks != null && !CollectionUtils.isEmpty(webhooks.getPostDeployHooks())) {
jobPool.submit(
new WebhookJob(webhooks.getPostDeployHooks(), deployBean, envBean));
jobPool.submit(new WebhookJob(webhooks.getPostDeployHooks(), deployBean));
LOG.info("Submitted post deploy hook job for deploy {}.", deployId);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
import com.pinterest.deployservice.common.CommonUtils;
import com.pinterest.deployservice.common.Constants;
import com.pinterest.deployservice.common.DeployInternalException;
import com.pinterest.deployservice.common.HTTPClient;
import com.pinterest.deployservice.common.StateMachines;
import com.pinterest.deployservice.common.WebhookDataFactory;
import com.pinterest.deployservice.dao.AgentDAO;
Expand All @@ -56,6 +55,7 @@
import com.pinterest.deployservice.db.DatabaseUtil;
import com.pinterest.deployservice.db.DeployQueryFilter;
import com.pinterest.deployservice.scm.SourceControlManagerProxy;
import com.pinterest.teletraan.universal.http.HttpClient;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
Expand Down Expand Up @@ -102,6 +102,7 @@ public class DeployHandler implements DeployHandlerInterface {
"This stage requires SOX builds. A private build cannot be used in a sox-compliant stage.";
static final String ERROR_STAGE_REQUIRES_SOX_BUILD_COMPLIANT_SOURCE =
"This stage requires SOX builds. The build must be from a sox-compliant source. Contact your sox administrators.";
private static final HttpClient httpClient = HttpClient.builder().build();

private final DeployDAO deployDAO;
private final EnvironDAO environDAO;
Expand All @@ -126,11 +127,9 @@ private final class NotifyJob implements Callable<Void> {
private final EnvironBean envBean;
private final DeployBean newDeployBean;
private final DeployBean oldDeployBean;
private final HTTPClient httpClient;
private final CommonHandler commonHandler;
private final String deployBoardUrlPrefix;
private final String changeFeedUrl;
private static final int RETRIES = 3;

public NotifyJob(
EnvironBean envBean,
Expand All @@ -142,7 +141,6 @@ public NotifyJob(
this.envBean = envBean;
this.newDeployBean = newDeployBean;
this.oldDeployBean = oldDeployBean;
this.httpClient = new HTTPClient();
this.commonHandler = commonHandler;
this.deployBoardUrlPrefix = deployBoardUrlPrefix;
this.changeFeedUrl = changeFeedUrl;
Expand All @@ -165,7 +163,7 @@ private void updateChangeFeed() {
envBean.getExternal_id());
Map<String, String> headers = new HashMap<>();
headers.put("Content-Type", "application/json");
httpClient.post(changeFeedUrl, feedPayload, headers, RETRIES);
httpClient.post(changeFeedUrl, feedPayload, headers);
LOG.info("Send change feed {} to {} successfully", feedPayload, changeFeedUrl);
} catch (Exception e) {
LOG.error(
Expand Down Expand Up @@ -397,7 +395,7 @@ String internalDeploy(EnvironBean envBean, DeployBean deployBean) throws Excepti
EnvWebHookBean webhook =
dataHandler.getDataById(envBean.getWebhooks_config_id(), WebhookDataFactory.class);
if (webhook != null && !CollectionUtils.isEmpty(webhook.getPreDeployHooks())) {
jobPool.submit(new WebhookJob(webhook.getPreDeployHooks(), deployBean, envBean));
jobPool.submit(new WebhookJob(webhook.getPreDeployHooks(), deployBean));
}

LOG.info("Submitted notify job for deploy {}", deployId);
Expand Down
Loading

0 comments on commit df4cc0d

Please sign in to comment.