ferenc-csaky commented on code in PR #26348: URL: https://github.com/apache/flink/pull/26348#discussion_r2014198008
########## flink-metrics/flink-metrics-otel/src/main/resources/META-INF/NOTICE: ########## @@ -6,31 +6,21 @@ The Apache Software Foundation (http://www.apache.org/). This project bundles the following dependencies under the Apache Software License 2.0. (http://www.apache.org/licenses/LICENSE-2.0.txt) -- io.opentelemetry:opentelemetry-sdk:1.30.1 -- io.opentelemetry:opentelemetry-api:1.30.1 -- io.opentelemetry:opentelemetry-context:1.30.1 -- io.opentelemetry:opentelemetry-sdk-common:1.30.1 -- io.opentelemetry:opentelemetry-sdk-trace:1.30.1 -- io.opentelemetry:opentelemetry-sdk-metrics:1.30.1 -- io.opentelemetry:opentelemetry-extension-incubator:1.30.1-alpha -- io.opentelemetry:opentelemetry-sdk-logs:1.30.1 -- io.opentelemetry:opentelemetry-api-events:1.30.1-alpha -- io.opentelemetry:opentelemetry-semconv:1.30.1-alpha -- io.opentelemetry:opentelemetry-exporter-logging:1.30.1 -- io.opentelemetry:opentelemetry-sdk-extension-autoconfigure-spi:1.30.1 -- io.opentelemetry:opentelemetry-exporter-otlp:1.30.1 -- io.opentelemetry:opentelemetry-exporter-otlp-common:1.30.1 -- io.opentelemetry:opentelemetry-exporter-common:1.30.1 -- io.opentelemetry:opentelemetry-exporter-sender-okhttp:1.30.1 -- io.opentelemetry:opentelemetry-sdk-testing:1.30.1 -- io.opentelemetry:opentelemetry-extension-incubator:1.36.0-alpha -- io.opentelemetry.proto:opentelemetry-proto:1.0.0-alpha -- io.opentelemetry:opentelemetry-api-common:0.13.1 -- io.opentelemetry:opentelemetry-api-incubator:1.38.0-alpha -- io.opentelemetry:opentelemetry-sdk-extension-autoconfigure:1.30.1 -- io.opentelemetry:opentelemetry-bom:1.38.0 -- com.squareup.okhttp3:okhttp:4.10.0 -- com.squareup.okio:okio-jvm:3.0.0 -- org.jetbrains.kotlin:kotlin-stdlib:1.6.20 -- org.jetbrains:annotations:17.0.0 -- org.json:json:20240303 +- io.opentelemetry:opentelemetry-sdk:1.30.0 Review Comment: I see that based on the given version in the POM this NOTICE was actually wrong, but since we already touch this, IMO it would make sense to bump it to `1.30.1`. ########## flink-metrics/flink-metrics-otel/pom.xml: ########## @@ -143,4 +143,39 @@ under the License. </dependency> </dependencies> + + <build> Review Comment: I do not think it is a realistic thing to write an example to test some dependency bundling. If that's something we'd like to shoot for, there are probably hundreds of possible places where it could be justified, but I think we do not even have the resource for much less. Providing an example for OTEL hands down would be a good thing to have, but I do not think it should be a part/requirement for this fix. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
