spring-attic / core Goto Github PK

Make kafka apps default to 0.10.1.1

The "app-starters-micrometer-common" for 2.0.0.RC1 has changed significantly, since the initial 1.3.x port. Resulting in new tag name convention (dot based), removed tags (e.g. clusterName) and support for CF tags.
This is a follow up of #38

1. In order to migrate testing to use the new SI Test Binder, it is necessary to move spring-cloud-stream-test-support to core-dependencies. Since it is present in the parent pom it gets pulled in to all app starters even though it is declared optional. This breaks the SI Test Binder when used in a Test since it is also test scope.

2. Since many app starters implicitly depend on spring-cloud-stream-test-support, the moved dependency should no longer be declared as optional. This should be restored to optional when the app starters start to migrate to the test binder.

NOTE. In the mean time, app starters that use the SI test binder need to exclude spring-cloud-stream-test-support

<dependency>
	<groupId>org.springframework.cloud.stream.app</groupId>
	<artifactId>app-starters-core-dependencies</artifactId>
	<version>2.0.0.BUILD-SNAPSHOT</version>
	<type>pom</type>
	<scope>import</scope>
	<exclusions>
		<exclusion>
			<groupId>org.springframework.cloud</groupId>
			<artifactId>spring-cloud-stream-test-support</artifactId>
		</exclusion>
	</exclusions>
</dependency>

3. The SI test binder should be added to core as an optional dependency and explicitly declared in the app starter if necessary.

The time is a reserved keyword for InfuxDB tag values. As a result the the time app starters fails to register its name as a metrics application.name tag:

2019-01-12 13:09:56.410 ERROR 25728 --- [trics-publisher] i.micrometer.influx.InfluxMeterRegistry  : failed to send metrics to influx

java.lang.IllegalArgumentException: 'time' is an invalid tag value in InfluxDB
        at io.micrometer.influx.InfluxNamingConvention.tagValue(InfluxNamingConvention.java:66) ~[micrometer-registry-influx-1.1.1.jar!/:1.1.1]
        at io.micrometer.core.instrument.Meter$Id.lambda$getConventionTags$0(Meter.java:330) ~[micrometer-core-1.1.1.jar!/:1.1.1]
        at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193) ~[na:1.8.0_152]

Quick work around is to rename the time app starter name to time2 for example.
Better solution though is customize the meter registry so it renames the time tag value to atime for InfluxDB.

Child of spring-cloud/spring-cloud-dataflow#2601

The spring-cloud-stream-metrics is obsolete in 2.0.0. Furthermore it drags in Spring Boot 1.5.x metrics dependencies casing Class Not Found exceptions.
This is a blocker for the Darwin.M1 release

Currently the task name is a required property, statically configured.

As a developer, I'd like to add a new app-starters-metadata-store module, so we can host auto-configuration code for a variety of MetadataStore.

Storage options:

• DynamoDB
• Gemfire
• Redis
• JDBC
• MongoDB
• ZK
• Hazelcast

Add the prometheus.rsocket metrics registry to enable Stream App starters use the Prometheus Rsocket Proxy instead of custom build Service Discovery mechanism.

As a developer, I'd like to upgrade kafka-binder to be compatible with 0.10 release of Apache Kafka as a default.

• FVC
• HTTP
• Gemfire
• AWS Apps
• Mongo
• Router [https://github.com/spring-attic/app-starters-release/issues/163]
• Tensorflow
• Intializr

Also, check for point releases in:

• Spring Cloud Build
• SCSt

@tzolov commented on Fri Mar 20 2020

The prometheus metrics for Fahrenheit apps is broken

The original idea is in the Tensorflow processor PR: spring-attic/tensorflow#1 (comment).

Since we don't test now spring-configuration-metadata-whitelist.properties in the app starter we don't know if that is correct for target integration system like SCDF.

Enable security for all the actuator endpoints.

Use SpEL to all message contents to be passed to tasks as args. For example:

file --spring.cloud.stream.function.definition=taskLaunchRequest --task.launch.request.task-name=foo --task.launch.request.arg-expressions='localFilePath=' + payload

Move https://github.com/spring-cloud-stream-app-starters/load-generator/blob/master/spring-cloud-starter-stream-source-load-generator/src/main/java/org/springframework/cloud/stream/app/load/generator/source/ContentTypeEnvironmentPostProcessor.java so it can be shared across other app projects needing the same functionality

This causes to have all the jars for testing in the target generated uber jar for the starter application.

Looks suspicious, but I won't mind if you find some argument and explain at least for me why we need them here.

Thanks

This has side effects for green field apps

The upgrade to Boot from 2.1.3 to 2.1.6, Fishtown.SR2 and Greenwich.SR2 is compulsory foor enabling the Prometheus RSocket Proxy micrometer (doesn't work for Boot versions below 2.1.5) but it seems to causes errors on CF environments (below). Looks like problem is with spring-cloud-spring-service-connector-2.0.5.RELEASE.jar derived from spring-boot-2.1.6.jar

Above changes don't cause issues on Local, minikube and GKE/k8s

This is the log from CF deploying time source BS:

2019-09-14T13:30:04.449+02:00 [APP/PROC/WEB/0] [OUT] java.lang.NoClassDefFoundError: org/springframework/jdbc/datasource/DelegatingDataSource
2019-09-14T13:30:04.449+02:00 [APP/PROC/WEB/0] [OUT] at java.lang.ClassLoader.defineClass1(Native Method) ~[na:1.8.0_222]
2019-09-14T13:30:04.449+02:00 [APP/PROC/WEB/0] [OUT] at java.lang.ClassLoader.defineClass(ClassLoader.java:763) ~[na:1.8.0_222]
2019-09-14T13:30:04.449+02:00 [APP/PROC/WEB/0] [OUT] at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:142) ~[na:1.8.0_222]
2019-09-14T13:30:04.449+02:00 [APP/PROC/WEB/0] [OUT] at java.net.URLClassLoader.defineClass(URLClassLoader.java:468) ~[na:1.8.0_222]
2019-09-14T13:30:04.449+02:00 [APP/PROC/WEB/0] [OUT] at java.net.URLClassLoader.access$100(URLClassLoader.java:74) ~[na:1.8.0_222]
2019-09-14T13:30:04.449+02:00 [APP/PROC/WEB/0] [OUT] at java.net.URLClassLoader$1.run(URLClassLoader.java:369) ~[na:1.8.0_222]
2019-09-14T13:30:04.449+02:00 [APP/PROC/WEB/0] [OUT] at java.net.URLClassLoader$1.run(URLClassLoader.java:363) ~[na:1.8.0_222]
2019-09-14T13:30:04.449+02:00 [APP/PROC/WEB/0] [OUT] at java.security.AccessController.doPrivileged(Native Method) ~[na:1.8.0_222]
2019-09-14T13:30:04.449+02:00 [APP/PROC/WEB/0] [OUT] at java.net.URLClassLoader.findClass(URLClassLoader.java:362) ~[na:1.8.0_222]
2019-09-14T13:30:04.449+02:00 [APP/PROC/WEB/0] [OUT] at java.lang.ClassLoader.loadClass(ClassLoader.java:424) ~[na:1.8.0_222]
2019-09-14T13:30:04.449+02:00 [APP/PROC/WEB/0] [OUT] at org.springframework.boot.loader.LaunchedURLClassLoader.loadClass(LaunchedURLClassLoader.java:92) ~[app/:na]

The markersJson = false means we don't want to convert FileMarker to JSON and just want to send it as serialized, but since we supply "text/plain" in the FileUtils:

flowBuilder.enrichHeaders(Collections.<String, Object>singletonMap(MessageHeaders.CONTENT_TYPE,
					"text/plain"))
					.split(new FileSplitter(true, withMarkers, fileConsumerProperties.getMarkersJson()));

the org.springframework.cloud.stream.converter.ObjectStringMessageConverter does its trick
and just call Object.toString() on our FileMarker instead of expected serialization.

And even if we would serialize it manually, that's going to be confusing on the consumer side since contentType == text/plain.

Another way might be like conditional header enricher and override the contentType header to the application/x-java-serialized-object if payload is FileMarker.

On the consumer side you can have conditional @StreamListener to handle string with one method
and FileMarkers with another.

We will be removing CF connector library from the apps.

As a developer, I'd like to review the patching procedure and as well as the customization steps for correctness.

See discussion here for background.

• Ship the out-of-the box applications with the following Micrometer dependencies in the classpath

  • Prometheus
  • InfluxDB
  • Datadog.

• All configured micrometer registries should be disabled by default, e.g.

--management.metrics.export.datadog.enabled=false
--management.metrics.export.influx.enabled=false
--management.metrics.export.prometheus.enabled=false

• Expose the Prometheus actuator endpoint (e.g. /actuator/prometheus/):

--management.endpoints.web.exposure.include=prometheus

Child of spring-cloud/spring-cloud-dataflow#2601

I think we should update org.springframework.cloud:spring-cloud-build version to 1.2.1.RELEASE

Adding a declarative Micrometer Tags injection utilities, common for all app starters.

The idea is to provide the same fixed/expression based tags enhancement functionality available in the Counter to all app starters.

For example, any app starter should be able to register additional Micrometer tags, with values dynamically resolved from the Message objects using SpEL expressions:

analytics.tag.expression.author=#jsonPath(payload,'$..author')

As a user, I'd like to monitor KStream apps in SCDF, so when including the stream-apps-micrometer-common in the CP, I hope to also automatically parse all the various KStreams binding types via this dependency.

Currently, depending on the FileReadingMode mode the FileUtils sets the following content types:

• application/octet-stream for content mode
• text/plain for lines mode

The ref mode though ( https://goo.gl/Xt8K3F ) is left to be set to the default content type! While currently this happens to be application/json we can not guaranty it will not change in the future. Furthermore this assumption creates an unnecessary coupling between this code (and all apps that use it) and the core SCSt functionality. Instead we should set the content-type explicitly to application/json similar to the other two modes.

Really two separate issues but:

Sftp source already provides the ability to optionally transform the output to a Task Launch Request but this is a cross-cutting concern for any message producer. Provide a core/common module to support this feature without requiring a separate transformer.

Sftp source needs to copy remote files to a local directory in most situations. If running in cloudfoundry with the nfs (Volume Services) service bound, the local directory path should be auto-reconfigured to use the services nfs mount point. This is also a cross-cutting concern for any source that creates files in a local directory. The app configures a @Bean LocalDirectoryResolver using the default constructor and uses that to resolve the local directory path. The auto reconfiguration provides a BeanPostProcessor to replace this bean if it exists and the above conditions are true.

Create an auto-configuration to inject common tags for:

• stream name
• application name
• application instance
• application guid
• application type

With the common security configuration class SecurityCommonAutoConfiguration's DisableSecurityConfiguration using the default Order of WebSecurityConfigurerAdapter which is 100, the other WebSecurityConfigurerAdapter in the same application configuration conflicts.

For instance, the http source app doesn't start with this and through the following:

Caused by: java.lang.IllegalStateException: @Order on WebSecurityConfigurers must be unique. Order of 100 was already used on org.springframework.cloud.stream.app.http.source.HttpSourceConfiguration$HttpSourceSecurityConfiguration$$EnhancerBySpringCGLIB$$50356d4e@55562aa9, so it cannot be used on org.springframework.cloud.stream.app.security.common.SecurityCommonAutoConfiguration$DisableSecurityConfiguration$$EnhancerBySpringCGLIB$$57f5ab9a@3bd418e4 too.

By default only info and health endpoints permit an unauthorized access.
For accessing the /actuator/prometheus we should either

• surpass the security for that endpoint
• configure secure communication between the Prometheus server and the app starters.

Child of spring-cloud/spring-cloud-dataflow#2601

From @sabbyanandan on March 26, 2018 22:44

As a developer, I'd like to add actuator and webflux dependencies, so I can enable /bindings endpoint by default across all the apps.

Here's the relevant section in the docs.

Acceptance:

• Add required dependencies to the core
• Verify dependencies are added to the generated apps
• Enable /bindings via management.endpoints.web.exposure.include=bindings property in all the apps

Copied from original issue: spring-attic/app-starters-release#136

As a developer, I'd like to add "micrometer-spring-legacy and app-starters-micrometer-common dependencies to 1.3.x branch.

Acceptance:

• Port app-starters-micrometer-common module to 1.3.x
• Add necessary dependencies
• Make sure everything compiles and tests run successfully
• Release 1.3.3 just for this project

Addressing the following SCDF request: Issue#12933, Spring Boot has implemented a Common Tags feature that allows to declaratively add Common Tags using just application properties.

Review the current SpringCloudStreamMicrometerCommonTags to see if we can simplify it using the new boot feature.

Provide support for any component to output a TaskLaunchRequest via function composition.

The PollerMetadata defaultPoller bean created in org.springframework.cloud.stream.app.trigger.TriggerConfiguration has an issue around properly setting the max messages per poll. The code is comparing this.triggerProperties.getMaxMessages() > -1 but I believe it should be this.triggerProperties.getMaxMessages() >= -1. Allowing the max messages per poll to be set to -1 is a necessity for when a polling operation fetches multiple events and queues them up for retrieval allowing them all to be obtained without waiting for the polling period between each message.

As documented in Spring Integration:

Note there is no max-messages-per-poll specified. As you’ll learn later the identical poller configuration in the PollingConsumer (e.g., service-activator, filter, router etc.) would have a default value of -1 for max-messages-per-poll which means "execute poling task non-stop unless polling method returns null (e.g., no more Messages in the QueueChannel)" and then sleep for 1 second.
However in the SourcePollingChannelAdapter it is a bit different. The default value for max-messages-per-poll will be set to 1 by default unless you explicitly set it to a negative value (e.g., -1). It is done so to make sure that poller can react to a LifeCycle events (e.g., start/stop) and prevent it from potentially spinning in the infinite loop if the implementation of the custom method of the MessageSource has a potential to never return null and happened to be non-interruptible.
However if you are sure that your method can return null and you need the behavior where you want to poll for as many sources as available per each poll, then you should explicitly set max-messages-per-poll to a negative value.

http://docs.spring.io/spring-integration/docs/4.3.10.RELEASE/reference/html/messaging-channels-section.html#channel-adapter-namespace-inbound

As a developer, I'd like to extract add org/space from env-var and add it to cmmonTags Map, so we could use it to plot metrics associated with streams in CF's Org/Space combination.

Acceptance:
This needs to be done only for CF and that can be determined based on Spring Profile being cloud.