hlteoh37 commented on code in PR #1: URL: https://github.com/apache/flink-connector-prometheus/pull/1#discussion_r1447607637
########## amp-request-signer/pom.xml: ########## @@ -0,0 +1,63 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- +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. +--> +<project xmlns="http://maven.apache.org/POM/4.0.0"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd";> + <modelVersion>4.0.0</modelVersion> + <parent> + <groupId>org.apache.flink</groupId> + <artifactId>flink-prometheus</artifactId> + <version>1.0.0-SNAPSHOT</version> + </parent> + + <name>Flink : Connectors : Prometheus : AMP request signer</name> + <groupId>org.apache.flink.connector.prometheus</groupId> + <artifactId>amp-request-signer</artifactId> + + <properties> + <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> + <target.java.version>11</target.java.version> + <maven.compiler.source>${target.java.version}</maven.compiler.source> + <maven.compiler.target>${target.java.version}</maven.compiler.target> + + <flink.version>1.16.0</flink.version> Review Comment: Should we consider 1.17? Since the supported version is 1.17 + 1.18 now ########## amp-request-signer/pom.xml: ########## @@ -0,0 +1,63 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- +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. +--> +<project xmlns="http://maven.apache.org/POM/4.0.0"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd";> + <modelVersion>4.0.0</modelVersion> + <parent> + <groupId>org.apache.flink</groupId> + <artifactId>flink-prometheus</artifactId> + <version>1.0.0-SNAPSHOT</version> + </parent> + + <name>Flink : Connectors : Prometheus : AMP request signer</name> + <groupId>org.apache.flink.connector.prometheus</groupId> + <artifactId>amp-request-signer</artifactId> + + <properties> + <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> + <target.java.version>11</target.java.version> + <maven.compiler.source>${target.java.version}</maven.compiler.source> + <maven.compiler.target>${target.java.version}</maven.compiler.target> + + <flink.version>1.16.0</flink.version> + </properties> + + <dependencies> + <dependency> + <groupId>org.apache.flink</groupId> + <artifactId>flink-connector-prometheus</artifactId> + <version>${project.version}</version> + </dependency> Review Comment: IIUC, this should be `<scope>provided</scope>`! ########## pom.xml: ########## @@ -0,0 +1,82 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- +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. +--> +<project xmlns="http://maven.apache.org/POM/4.0.0"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd";> + <modelVersion>4.0.0</modelVersion> + + <groupId>org.apache.flink</groupId> + <artifactId>flink-prometheus</artifactId> Review Comment: Should we keep the same convention as other connector repos `flink-connector-prometheus-parent`? ########## amp-request-signer/src/main/java/org/apache/flink/connector/prometheus/sink/aws/AmazonManagedPrometheusWriteRequestSigner.java: ########## @@ -0,0 +1,94 @@ +/* + * 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.flink.connector.prometheus.sink.aws; + +import org.apache.flink.connector.prometheus.sink.PrometheusRequestSigner; +import org.apache.flink.util.Preconditions; + +import com.amazonaws.auth.AWSCredentials; +import com.amazonaws.auth.AWSSessionCredentials; +import com.amazonaws.auth.DefaultAWSCredentialsProviderChain; +import com.amazonaws.util.BinaryUtils; +import org.apache.commons.lang3.StringUtils; + +import java.net.MalformedURLException; +import java.net.URL; +import java.util.Map; + +/** Sing a Remote-Write request to Amazon Managed Service for Prometheus (AMP). */ +public class AmazonManagedPrometheusWriteRequestSigner implements PrometheusRequestSigner { + + private final URL remoteWriteUrl; + private final String awsRegion; + + /** + * Constructor. + * + * @param remoteWriteUrl URL of the remote-write endpoint + * @param awsRegion Region of the AMP workspace + */ + public AmazonManagedPrometheusWriteRequestSigner(String remoteWriteUrl, String awsRegion) { + Preconditions.checkArgument(StringUtils.isNotBlank(awsRegion)); Review Comment: Should we add a message here in the event of failure? ########## amp-request-signer/src/main/java/org/apache/flink/connector/prometheus/sink/aws/AWS4SignerBase.java: ########## @@ -0,0 +1,290 @@ +/* + * 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.flink.connector.prometheus.sink.aws; + +import com.amazonaws.util.BinaryUtils; + +import javax.crypto.Mac; +import javax.crypto.spec.SecretKeySpec; + +import java.io.UnsupportedEncodingException; +import java.net.URL; +import java.net.URLEncoder; +import java.security.MessageDigest; +import java.text.SimpleDateFormat; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.SimpleTimeZone; +import java.util.SortedMap; +import java.util.TreeMap; + +/** Common methods and properties for all AWS4 signer variants. */ +public abstract class AWS4SignerBase { + + /** SHA256 hash of an empty request body. */ + public static final String EMPTY_BODY_SHA256 = + "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"; + + public static final String UNSIGNED_PAYLOAD = "UNSIGNED-PAYLOAD"; + + public static final String SCHEME = "AWS4"; + public static final String ALGORITHM = "HMAC-SHA256"; + public static final String TERMINATOR = "aws4_request"; + + /** format strings for the date/time and date stamps required during signing. */ + public static final String ISO_8601_BASIC_FORMAT = "yyyyMMdd'T'HHmmss'Z'"; + + public static final String DATE_STRING_FORMAT = "yyyyMMdd"; + + protected URL endpointUrl; + protected String httpMethod; + protected String serviceName; + protected String regionName; + + protected final SimpleDateFormat dateTimeFormat; + protected final SimpleDateFormat dateStampFormat; + + /** + * Create a new AWS V4 signer. + * + * @param endpointUrl The service endpoint, including the path to any resource. + * @param httpMethod The HTTP verb for the request, e.g. GET. + * @param serviceName The signing name of the service, e.g. 's3'. + * @param regionName The system name of the AWS region associated with the endpoint, e.g. + * us-east-1. + */ + public AWS4SignerBase( + URL endpointUrl, String httpMethod, String serviceName, String regionName) { + this.endpointUrl = endpointUrl; + this.httpMethod = httpMethod; + this.serviceName = serviceName; + this.regionName = regionName; + + dateTimeFormat = new SimpleDateFormat(ISO_8601_BASIC_FORMAT); + dateTimeFormat.setTimeZone(new SimpleTimeZone(0, "UTC")); + dateStampFormat = new SimpleDateFormat(DATE_STRING_FORMAT); + dateStampFormat.setTimeZone(new SimpleTimeZone(0, "UTC")); Review Comment: Is there a reason we fix this as 0 UTC? Is there a scenario we would want to have a different timezone here? If so, maybe we can make this configurable / use the timezone of the host ########## prometheus-connector/src/main/java/org/apache/flink/connector/prometheus/sink/PrometheusRemoteWriteHttpRequestBuilder.java: ########## @@ -0,0 +1,70 @@ +/* + * 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.flink.connector.prometheus.sink; + +import org.apache.hc.client5.http.async.methods.SimpleHttpRequest; +import org.apache.hc.client5.http.async.methods.SimpleRequestBuilder; +import org.apache.hc.core5.http.ContentType; +import org.apache.hc.core5.http.HttpHeaders; + +import java.util.HashMap; +import java.util.Map; + +/** Builds the POST request to the Remote-Write endpoint for a given binary payload. */ +public class PrometheusRemoteWriteHttpRequestBuilder { + + private static final ContentType CONTENT_TYPE = ContentType.create("application/x-protobuf"); + + private static final Map<String, String> FIXED_HEADER = + Map.of( + HttpHeaders.CONTENT_ENCODING, + "snappy", + "X-Prometheus-Remote-Write-Version", + "0.1.0", + HttpHeaders.USER_AGENT, + "Flink-Prometheus/0.1.0" // TODO Prometheus requires a user-agent header. What Review Comment: As discussed offline, maybe we can make this configurable, and default to `Flink-Prometheus`? ########## msf-amp-example/README.md: ########## @@ -0,0 +1,68 @@ +## Sample application: Amazon Managed Service for Apache Flink and Amazon Managed Prometheus Review Comment: We shouldn't be putting company specific examples here. Should we consider using a non-MSF example here instead, and move this MSF-specific one elsewhere? ########## amp-request-signer/src/main/java/org/apache/flink/connector/prometheus/sink/aws/AWS4SignerBase.java: ########## @@ -0,0 +1,290 @@ +/* + * 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.flink.connector.prometheus.sink.aws; + +import com.amazonaws.util.BinaryUtils; + +import javax.crypto.Mac; +import javax.crypto.spec.SecretKeySpec; Review Comment: Is there a reason we use `javax.crypto.Mac` here? Do we know if this limits our java version etc? ########## amp-request-signer/src/main/java/org/apache/flink/connector/prometheus/sink/aws/AmazonManagedPrometheusWriteRequestSigner.java: ########## @@ -0,0 +1,94 @@ +/* + * 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.flink.connector.prometheus.sink.aws; + +import org.apache.flink.connector.prometheus.sink.PrometheusRequestSigner; +import org.apache.flink.util.Preconditions; + +import com.amazonaws.auth.AWSCredentials; +import com.amazonaws.auth.AWSSessionCredentials; +import com.amazonaws.auth.DefaultAWSCredentialsProviderChain; +import com.amazonaws.util.BinaryUtils; +import org.apache.commons.lang3.StringUtils; + +import java.net.MalformedURLException; +import java.net.URL; +import java.util.Map; + +/** Sing a Remote-Write request to Amazon Managed Service for Prometheus (AMP). */ Review Comment: nit: `Sign` We also might want to put an example usage in the docs here. ########## prometheus-connector/pom.xml: ########## @@ -0,0 +1,130 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- +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. +--> +<project xmlns="http://maven.apache.org/POM/4.0.0"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd";> + <modelVersion>4.0.0</modelVersion> + <parent> + <groupId>org.apache.flink</groupId> + <artifactId>flink-prometheus</artifactId> + <version>1.0.0-SNAPSHOT</version> + </parent> + + <name>Flink : Connectors : Prometheus</name> + <groupId>org.apache.flink</groupId> + <artifactId>flink-connector-prometheus</artifactId> + <packaging>jar</packaging> + + + <properties> + <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> + <target.java.version>11</target.java.version> + <maven.compiler.source>${target.java.version}</maven.compiler.source> + <maven.compiler.target>${target.java.version}</maven.compiler.target> + + <flink.version>1.16.2</flink.version> + <protobuf.version>3.22.2</protobuf.version> + <apache.httpclient.version>5.2.1</apache.httpclient.version> + </properties> + + <dependencies> + <dependency> + <groupId>org.apache.flink</groupId> + <artifactId>flink-streaming-java</artifactId> + <version>${flink.version}</version> + <scope>provided</scope> + </dependency> + <dependency> + <groupId>org.apache.flink</groupId> + <artifactId>flink-connector-base</artifactId> + <version>${flink.version}</version> + <scope>provided</scope> + </dependency> + + <!-- protobuf, snappy - for prometheus remote-write payload --> + <dependency> + <groupId>com.google.protobuf</groupId> + <artifactId>protobuf-java</artifactId> + <version>${protobuf.version}</version> + </dependency> + <dependency> + <groupId>com.google.protobuf</groupId> + <artifactId>protobuf-java-util</artifactId> + <version>${protobuf.version}</version> + </dependency> + <dependency> + <groupId>org.xerial.snappy</groupId> + <artifactId>snappy-java</artifactId> + <version>1.1.9.1</version> + </dependency> + + <!-- http client --> + <dependency> + <groupId>org.apache.httpcomponents.client5</groupId> + <artifactId>httpclient5</artifactId> + <version>${apache.httpclient.version}</version> + </dependency> + <dependency> + <groupId>org.apache.httpcomponents.core5</groupId> + <artifactId>httpcore5</artifactId> + <version>${apache.httpclient.version}</version> + </dependency> + + <!-- test --> + <dependency> + <groupId>org.apache.flink</groupId> + <artifactId>flink-test-utils-junit</artifactId> + <version>${flink.version}</version> + <scope>test</scope> + </dependency> + <dependency> + <groupId>com.github.tomakehurst</groupId> + <artifactId>wiremock-jre8</artifactId> + <version>2.35.0</version> + <scope>test</scope> + </dependency> + <dependency> + <groupId>org.mockito</groupId> + <artifactId>mockito-core</artifactId> + <version>4.5.1</version> + <scope>test</scope> + </dependency> + <dependency> + <groupId>org.awaitility</groupId> + <artifactId>awaitility</artifactId> + <version>4.2.0</version> + <scope>test</scope> + </dependency> + </dependencies> + + <build> + <plugins> + <plugin> + <groupId>org.apache.maven.plugins</groupId> + <artifactId>maven-compiler-plugin</artifactId> + <version>3.8.0</version> + <configuration> + <source>${target.java.version}</source> + <target>${target.java.version}</target> + </configuration> + </plugin> + </plugins> + </build> Review Comment: this might already come from the connector parent! We should check! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
