Add aws-firehose-receiver to support collecting AWS CloudWatch metric(OpenTelemetry format) #10300

pg-yang · 2023-01-21T05:59:54Z

If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #.
Update the CHANGES log.

Related issue : #9883

oap-server/server-starter/pom.xml

kezhenxu94

You are mixing firehose and firehouse, please check which one is correct and modify the wrong ones. And the title is even firehousr

docs/en/setup/backend/aws-firehouse-receiver.md

oap-server/server-receiver-plugin/aws-firehose-receiver/pom.xml

...rc/main/java/org/apache/skywalking/oap/server/receiver/aws/firehose/FirehoseHTTPHandler.java

...src/main/java/org/apache/skywalking/oap/server/receiver/otel/OtelMetricReceiverProvider.java

...rc/main/java/org/apache/skywalking/oap/server/receiver/aws/firehose/FirehoseHTTPHandler.java

wu-sheng · 2023-01-21T09:12:03Z

You are mixing firehose and firehouse, please check which one is correct and modify the wrong ones. And the title is even firehousr

The official name should be Amazon Kinesis Data Firehose. https://aws.amazon.com/cn/kinesis/data-firehose/

kezhenxu94 · 2023-01-21T11:39:22Z

And this is still based on the existing common http server, how do you plan to implement https for this plugin? I think you need to start an individual http server for this plugin and configure https-related things.

wu-sheng · 2023-01-21T11:53:13Z

And this is still based on the existing common http server, how do you plan to implement https for this plugin? I think you need to start an individual http server for this plugin and configure https-related things.

This should be a separate HTTP server out of core and sharing HTTP servers.

docs/en/setup/backend/aws-firehose-receiver.md

wu-sheng · 2023-01-22T00:37:01Z

docs/en/setup/backend/aws-firehose-receiver.md

+
+1. Only OpenTelemetry format is supported (refer to [Metric streams output formats](https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/CloudWatch-metric-streams-formats.html))
+2. Doesn't support HTTP Headers - Content-Encoding
+3. Only HTTPS could be accepted, you could directly enable TSL and set the receiver to listen 443, or put the receiver behind a gateway with HTTPS (refer to [Amazon Kinesis Data Firehose Delivery Stream HTTP Endpoint Delivery Specifications](https://docs.aws.amazon.com/firehose/latest/dev/httpdeliveryrequestresponse.html))


Is 443 or 13800 expected to set? I found inconsistent in the doc

The default port is 13800. But if users want to directly use the receiver, they have to set the port to 443, because of require of AWS

If AWS requires this, we should set 443 by default. As the HTTP server is off by OAP, we should be good.

443 requires root privilege, It will force the user to run OAP by root. So I choose another port as default and introduce that the port could be modified in the doc.

Does AWS support use custom port? This only makes sense if aws did.

Does AWS support use custom port? This only makes sense if aws did.

It makes sense when users run OAP behind a gateway. In my opinion it makes no sense to expose port 443 (https port) from OAP, it's hardly possible in reality. I'm still in doubt whether this is useful and practical in reality as they said Currently, only port 443 is supported for HTTP endpoint data delivery.

If we need a gateway, we should document it. But the reason is the key.

In this thread, two things we need to make sure.

443 is required or just default

whether a gatway is recommended, and why.

...rc/main/java/org/apache/skywalking/oap/server/receiver/aws/firehose/FirehoseHTTPHandler.java

...rg/apache/skywalking/oap/server/receiver/aws/firehose/AWSFirehoseReceiverModuleProvider.java

...org/apache/skywalking/oap/server/receiver/otel/otlp/OpenTelemetryMetricRequestProcessor.java

oap-server/server-starter/pom.xml

...rc/main/java/org/apache/skywalking/oap/server/receiver/aws/firehose/FirehoseHTTPHandler.java

pg-yang · 2023-01-22T09:16:17Z

2023-01-22 08:54:09,078 com.linecorp.armeria.server.logging.LoggingService 155 [armeria-eventloop-epoll-8-132] WARN  [] - [sreqId=fac070d5, chanId=b05ca0f3, raddr=172.18.0.4:37920, laddr=172.18.0.3:9411][h1c://5ab144efd47b/api/v2/spans#POST] Request: {startTime=2023-01-22T08:54:09.070Z(1674377649070048), length=282B, duration=6317µs(6317993ns), scheme=none+h1c, name=collectV2JsonSpans, headers=[:method=POST, :path=/api/v2/spans, :scheme=http, b3=0, content-type=application/json, content-encoding=gzip, user-agent=Java/15.0.1, host=oap:9411, accept=text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2, content-length=282]}
2023-01-22 08:54:09,078 com.linecorp.armeria.server.logging.LoggingService 186 [armeria-eventloop-epoll-8-132] WARN  [] - [sreqId=fac070d5, chanId=b05ca0f3, raddr=172.18.0.4:37920, laddr=172.18.0.3:9411][h1c://5ab144efd47b/api/v2/spans#POST] Response: {startTime=2023-01-22T08:54:09.075Z(1674377649075633), length=47B, duration=2051µs(2051614ns), totalDuration=7638µs(7638019ns), cause=io.netty.handler.codec.compression.DecompressionException: Input is not in the GZIP format, headers=[:status=500, content-type=text/plain; charset=utf-8, content-length=47]}
io.netty.handler.codec.compression.DecompressionException: Input is not in the GZIP format
	at io.netty.handler.codec.compression.JdkZlibDecoder.readGZIPHeader(JdkZlibDecoder.java:324) ~[netty-codec-4.1.86.Final.jar:4.1.86.Final]
	at io.netty.handler.codec.compression.JdkZlibDecoder.decode(JdkZlibDecoder.java:213) ~[netty-codec-4.1.86.Final.jar:4.1.86.Final]
	at io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:529) ~[netty-codec-4.1.86.Final.jar:4.1.86.Final]
	at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:468) ~[netty-codec-4.1.86.Final.jar:4.1.86.Final]
	at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:290) ~[netty-codec-4.1.86.Final.jar:4.1.86.Final]
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444) ~[netty-transport-4.1.86.Final.jar:4.1.86.Final]

It seems armeria zipkin client defines content-encoding=gzip in headers, but the request data isn't gzip format. I will confirm this issue.

wu-sheng · 2023-01-22T09:21:56Z

Why suddenly are we talking about Zipkin?

pg-yang · 2023-01-22T09:25:32Z

Because I added ServerBuilder#decorator(DecodingService.newDecorator()) to support content-encoding in this PR. This change makes CICD wrong

4c1df86#diff-b034fcbcccfb37d631c81c5c140b518d5b708a395cfa0b24162f492ddb749ddbR76

wu-sheng · 2023-01-22T10:03:10Z

I think we don't need to change the default behavior.
Strick content-encoding failure is not a new thing in Zipkin. You just need to add this to awsreceiver only.

kezhenxu94 · 2023-01-22T11:02:17Z

Because I added ServerBuilder#decorator(DecodingService.newDecorator()) to support content-encoding in this PR. This change makes CICD wrong

Please note that Zipkin receiver decodes the content by itself, if you add a decoding service decorator to the http server, you should remove the one in Zipkin receiver, otherwise the content is decoded twice, which is obviously wrong.

I think it's good to add the decoding service to the http server by default, normally an http server should take care of the encoding of the content according to the header out of box.

@pg-yang a patch can be applied directly (click to expand):

diff --git a/oap-server/server-library/library-server/src/main/java/org/apache/skywalking/oap/server/library/server/http/HTTPServer.java b/oap-server/server-library/library-server/src/main/java/org/apache/skywalking/oap/server/library/server/http/HTTPServer.java
index 673163a6b1..67de3c2ca0 100644
--- a/oap-server/server-library/library-server/src/main/java/org/apache/skywalking/oap/server/library/server/http/HTTPServer.java
+++ b/oap-server/server-library/library-server/src/main/java/org/apache/skywalking/oap/server/library/server/http/HTTPServer.java
@@ -72,10 +72,9 @@ public class HTTPServer implements Server {
                     return HttpResponse.of(HttpStatus.METHOD_NOT_ALLOWED);
                 }
                 return delegate.serve(ctx, req);
-            }).decorator(LoggingService.newDecorator());
-        if (config.isEnableContentEncoding()) {
-            sb.decorator(DecodingService.newDecorator());
-        }
+            })
+            .decorator(DecodingService.newDecorator())
+            .decorator(LoggingService.newDecorator());
         if (config.isEnableTLS()) {
             sb.https(new InetSocketAddress(
                     config.getHost(),
diff --git a/oap-server/server-library/library-server/src/main/java/org/apache/skywalking/oap/server/library/server/http/HTTPServerConfig.java b/oap-server/server-library/library-server/src/main/java/org/apache/skywalking/oap/server/library/server/http/HTTPServerConfig.java
index 1693cb71a5..9d9daa1356 100644
--- a/oap-server/server-library/library-server/src/main/java/org/apache/skywalking/oap/server/library/server/http/HTTPServerConfig.java
+++ b/oap-server/server-library/library-server/src/main/java/org/apache/skywalking/oap/server/library/server/http/HTTPServerConfig.java
@@ -45,8 +45,4 @@ public class HTTPServerConfig {
 
     private String tlsKeyPath;
     private String tlsCertChainPath;
-
-    @Builder.Default
-    private boolean enableContentEncoding = false;
-
 }
diff --git a/oap-server/server-receiver-plugin/zipkin-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/zipkin/handler/UnzippingBytesRequestConverter.java b/oap-server/server-receiver-plugin/zipkin-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/zipkin/handler/UnzippingBytesRequestConverter.java
deleted file mode 100644
index 412ccac7d3..0000000000
--- a/oap-server/server-receiver-plugin/zipkin-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/zipkin/handler/UnzippingBytesRequestConverter.java
+++ /dev/null
@@ -1,41 +0,0 @@
-/*
- * 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.skywalking.oap.server.receiver.zipkin.handler;
-
-import com.linecorp.armeria.common.AggregatedHttpRequest;
-import com.linecorp.armeria.common.HttpData;
-import com.linecorp.armeria.common.HttpHeaderNames;
-import com.linecorp.armeria.common.encoding.StreamDecoderFactory;
-import com.linecorp.armeria.server.ServiceRequestContext;
-
-final class UnzippingBytesRequestConverter {
-
-    static HttpData convertRequest(ServiceRequestContext ctx, AggregatedHttpRequest request) {
-        String encoding = request.headers().get(HttpHeaderNames.CONTENT_ENCODING);
-        HttpData content = request.content();
-        if (!content.isEmpty() && encoding != null && encoding.contains("gzip")) {
-            content = StreamDecoderFactory.gzip().newDecoder(ctx.alloc()).decode(content);
-            if (content.isEmpty()) {
-                content.close();
-                throw new IllegalArgumentException("Cannot unzip request content bytes");
-            }
-        }
-        return content;
-    }
-}
diff --git a/oap-server/server-receiver-plugin/zipkin-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/zipkin/handler/ZipkinSpanHTTPHandler.java b/oap-server/server-receiver-plugin/zipkin-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/zipkin/handler/ZipkinSpanHTTPHandler.java
index 2acaa9d848..ea5cadcf12 100644
--- a/oap-server/server-receiver-plugin/zipkin-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/zipkin/handler/ZipkinSpanHTTPHandler.java
+++ b/oap-server/server-receiver-plugin/zipkin-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/zipkin/handler/ZipkinSpanHTTPHandler.java
@@ -100,7 +100,7 @@ public class ZipkinSpanHTTPHandler {
                                 final HttpRequest req) {
         final HistogramMetrics.Timer timer = histogram.createTimer();
         final HttpResponse response = HttpResponse.from(req.aggregate().thenApply(request -> {
-            try (final HttpData httpData = UnzippingBytesRequestConverter.convertRequest(ctx, request)) {
+            try (final HttpData httpData = request.content()) {
                 final List<Span> spanList = decoder.decodeList(httpData.byteBuf().nioBuffer());
                 spanForward.send(spanList);
                 return HttpResponse.of(HttpStatus.OK);

oap-server/server-starter/src/main/resources/application.yml

...src/main/java/org/apache/skywalking/oap/server/receiver/otel/OtelMetricReceiverProvider.java

...rg/apache/skywalking/oap/server/receiver/aws/firehose/AWSFirehoseReceiverModuleProvider.java

…(OpenTelemetry format)

wu-sheng

Generally good for this version. Pend on @kezhenxu94 to check details.

docs/en/setup/backend/configuration-vocabulary.md

docs/en/setup/backend/aws-firehose-receiver.md

Co-authored-by: kezhenxu94 <kezhenxu94@apache.org>

docs/en/setup/backend/configuration-vocabulary.md

Co-authored-by: kezhenxu94 <kezhenxu94@apache.org>

pg-yang requested review from kezhenxu94, wu-sheng and yswdqz January 21, 2023 06:00

wu-sheng reviewed Jan 21, 2023

View reviewed changes

oap-server/server-starter/pom.xml Show resolved Hide resolved

kezhenxu94 requested changes Jan 21, 2023

View reviewed changes

pg-yang changed the title ~~Add aws-firehousr-receiver to support collecting AWS CloudWatch metric(OpenTelemetry format)~~ Add aws-firehose-receiver to support collecting AWS CloudWatch metric(OpenTelemetry format) Jan 21, 2023

wu-sheng reviewed Jan 21, 2023

View reviewed changes

...rc/main/java/org/apache/skywalking/oap/server/receiver/aws/firehose/FirehoseHTTPHandler.java Outdated Show resolved Hide resolved

wu-sheng added this to the 9.4.0 milestone Jan 21, 2023

wu-sheng added backend OAP backend related. feature New feature labels Jan 21, 2023

wu-sheng reviewed Jan 22, 2023

View reviewed changes

kezhenxu94 requested changes Jan 22, 2023

View reviewed changes

...rc/main/java/org/apache/skywalking/oap/server/receiver/aws/firehose/FirehoseHTTPHandler.java Outdated Show resolved Hide resolved

kezhenxu94 reviewed Jan 22, 2023

View reviewed changes

oap-server/server-starter/src/main/resources/application.yml Outdated Show resolved Hide resolved

kezhenxu94 reviewed Jan 22, 2023

View reviewed changes

...src/main/java/org/apache/skywalking/oap/server/receiver/otel/OtelMetricReceiverProvider.java Outdated Show resolved Hide resolved

kezhenxu94 reviewed Jan 22, 2023

View reviewed changes

...rg/apache/skywalking/oap/server/receiver/aws/firehose/AWSFirehoseReceiverModuleProvider.java Show resolved Hide resolved

pg-yang mentioned this pull request Jan 23, 2023

Remove handler scan in otel receiver, manual initialization instead #10303

Merged

2 tasks

Add aws-firehose-receiver to support collecting AWS CloudWatch metric…

e82558e

…(OpenTelemetry format)

wu-sheng reviewed Jan 23, 2023

View reviewed changes

docs/en/setup/backend/configuration-vocabulary.md Outdated Show resolved Hide resolved

pg-yang added 3 commits January 23, 2023 17:07

add OpenTelemetryMetricRequestProcessor

2a9a325

polish firehose doc

681ca1d

more accurate exception -> InvalidProtocolBufferException

56a5000

kezhenxu94 previously approved these changes Jan 23, 2023

View reviewed changes

docs/en/setup/backend/aws-firehose-receiver.md Outdated Show resolved Hide resolved

Update docs/en/setup/backend/aws-firehose-receiver.md

7d5f545

Co-authored-by: kezhenxu94 <kezhenxu94@apache.org>

pg-yang dismissed kezhenxu94’s stale review via 7d5f545 January 23, 2023 16:38

kezhenxu94 reviewed Jan 24, 2023

View reviewed changes

docs/en/setup/backend/configuration-vocabulary.md Outdated Show resolved Hide resolved

Update docs/en/setup/backend/configuration-vocabulary.md

3f0ddac

Co-authored-by: kezhenxu94 <kezhenxu94@apache.org>

kezhenxu94 approved these changes Jan 24, 2023

View reviewed changes

wu-sheng approved these changes Jan 24, 2023

View reviewed changes

wu-sheng merged commit 29556a0 into apache:master Jan 24, 2023

kezhenxu94 mentioned this pull request Feb 5, 2023

Polish aws-firehose-receiver to adapt existing OTEL proto #10343

Merged

2 tasks

Add aws-firehose-receiver to support collecting AWS CloudWatch metric(OpenTelemetry format) #10300

Add aws-firehose-receiver to support collecting AWS CloudWatch metric(OpenTelemetry format) #10300

Uh oh!

Conversation

pg-yang commented Jan 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

kezhenxu94 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wu-sheng commented Jan 21, 2023

Uh oh!

kezhenxu94 commented Jan 21, 2023

Uh oh!

wu-sheng commented Jan 21, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wu-sheng Jan 22, 2023

Choose a reason for hiding this comment

Uh oh!

pg-yang Jan 22, 2023

Choose a reason for hiding this comment

Uh oh!

wu-sheng Jan 22, 2023

Choose a reason for hiding this comment

Uh oh!

pg-yang Jan 22, 2023

Choose a reason for hiding this comment

Uh oh!

wu-sheng Jan 22, 2023

Choose a reason for hiding this comment

Uh oh!

kezhenxu94 Jan 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wu-sheng Jan 22, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pg-yang commented Jan 22, 2023

Uh oh!

wu-sheng commented Jan 22, 2023

Uh oh!

pg-yang commented Jan 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wu-sheng commented Jan 22, 2023

Uh oh!

kezhenxu94 commented Jan 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wu-sheng left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

pg-yang commented Jan 21, 2023 •

edited

Loading

kezhenxu94 left a comment •

edited

Loading

kezhenxu94 Jan 22, 2023 •

edited

Loading

pg-yang commented Jan 22, 2023 •

edited

Loading

kezhenxu94 commented Jan 22, 2023 •

edited

Loading