You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2022/09/13 19:47:15 UTC
[GitHub] [nifi] greyp9 commented on a diff in pull request #6390: NIFI-10471 Document and apply standard deprecation logging
greyp9 commented on code in PR #6390:
URL: https://github.com/apache/nifi/pull/6390#discussion_r969823088
##########
nifi-docs/src/main/asciidoc/developer-guide.adoc:
##########
@@ -2637,19 +2637,83 @@ logged to help avoid this bad practice.
[[deprecation]]
-== Deprecating a Component
+== Deprecating Components and Features
+Deprecating features is an important part of the software development lifecycle, providing an upgrade path for
+developers and users to follow.
+
+Apache NiFi follows the link:https://semver.org[Semantic Versioning Specification 2.0.0^] for features identified as
+part of the public version contract according to
+link:https://cwiki.apache.org/confluence/display/NIFI/Version+Scheme+and+API+Compatibility[Version Scheme]
+documentation.
+
+Components and features that fit under the public version contract require deprecation marking prior to removal. This
+approach allows developers to implement changes as part of minor version upgrades, in preparation for future removal
+of features in a subsequent major version.
Review Comment:
So are we saying here that removal of features would *only* take place in the context of a major version upgrade?
##########
nifi-docs/src/main/asciidoc/developer-guide.adoc:
##########
@@ -2637,19 +2637,83 @@ logged to help avoid this bad practice.
[[deprecation]]
-== Deprecating a Component
+== Deprecating Components and Features
+Deprecating features is an important part of the software development lifecycle, providing an upgrade path for
+developers and users to follow.
+
+Apache NiFi follows the link:https://semver.org[Semantic Versioning Specification 2.0.0^] for features identified as
+part of the public version contract according to
+link:https://cwiki.apache.org/confluence/display/NIFI/Version+Scheme+and+API+Compatibility[Version Scheme]
+documentation.
+
+Components and features that fit under the public version contract require deprecation marking prior to removal. This
+approach allows developers to implement changes as part of minor version upgrades, in preparation for future removal
+of features in a subsequent major version.
+
+=== Component Deprecation
+
Sometimes it may be desirable to deprecate a component. Whenever this occurs the developer may use the
`@DeprecationNotice` annotation to indicate that a component has been deprecated, allowing the developer
- to describe a reason for the deprecation and suggest alternative components. An example of how to do this can
+ to describe a `reason` for the deprecation and suggest alternative components. An example of how to do this can
be found below:
[source, java]
----
- @DeprecationNotice(alternatives = {ListenSyslog.class}, classNames = {"org.apache.nifi.processors.standard.ListenRELP"}, reason = "Technology has been superseded", )
- public class ListenOldProtocol extends AbstractProcessor {
+@DeprecationNotice(
+ reason = "Legacy Protocol has been superseded",
+ alternatives = {ListenSyslog.class},
+ classNames = {"org.apache.nifi.processors.standard.ListenRELP"}
+)
+public class ListenLegacyProtocol extends AbstractProcessor {}
+----
+The `alternatives` property can be used to define an array of recommended replacement Components, while `classNames`
+can be used to represent similar content through an array of class name strings.
+
+Adding the `@DeprecationNotice` annotation renders a warning message in generated documentation and also logs the
+following warning when the Flow Configuration includes the component:
+
+----
+Added Deprecated Component ListenLegacyProtocol[id=929a52c7-1e3e-423e-b303-6ca2ef657617] See alternatives [ListenSyslog,ListenRELP]
+----
+
+=== Feature Deprecation
+
+Deprecating features includes changing component configuration strategies, introducing new repository classes, and
+refactoring a Controller Service interface. Removing component properties can create invalid Flow Configurations after
+upgrading, and removing public methods from a Controller Service interface can break components compiled against
+previous versions. For these reasons, introducing new properties and methods must include a deprecation strategy that
+supports compatibility when upgrading from one minor version to another.
+
+Annotating methods and properties with the Java `@Deprecated` annotation provides a warning to software developers, but
Review Comment:
+1, ... this suggests a good strategy for deprecation, `@Deprecated` for developers, and deprecation logging for users
##########
nifi-docs/src/main/asciidoc/administration-guide.adoc:
##########
@@ -182,6 +182,41 @@ Antivirus software can take a long time to scan large directories and the numero
* `provenance_repository`
* `state`
+[[logging_configuration]]
+== Logging Configuration
+NiFi uses link:https://logback.qos.ch/[logback^] as the runtime logging implementation. The `conf` directory contains a
+standard `logback.xml` configuration with default appender and level settings. The
+link:https://logback.qos.ch/manual/index.html[logback manual] provides a complete reference of available options.
+
+=== Standard Log Files
+The standard logback configuration includes the following appender definitions and associated log files:
+
+[options="header"]
+|=========================
+| File | Description
+| `nifi-app.log` | Application log containing framework and component messages
+| `nifi-bootstrap.log` | Bootstrap log containing startup and shutdown messages
+| `nifi-deprecation.log` | Deprecation log containing warnings for deprecated components and features
+| `nifi-request.log` | HTTP request log containing user interface and REST API access messages
+| `nifi-user.log` | User log containing authentication and authorization messages
+|=========================
+
+=== Deprecation Logging
+The `nifi-deprecation.log` contains warning messages describing components and features that will be removed in
+subsequent versions. Deprecation warnings should be evaluated and addressed to avoid breaking changes when upgrading to
+a new major version. Resolving deprecation warnings involves upgrading to new components, changing component property
+settings, or refactoring custom component classes.
+
Review Comment:
Suggest additional clarifying wording here along the lines of:
- The deprecation log should be checked before upgrading to the next minor version.
- To upgrade several minor versions, the deprecation notices in the release notes should be checked for relevant content.
- To upgrade to the next major version, first upgrade to the latest minor version, and run the flow for some period of time to check for deprecation logs indicating needed corrections.
##########
nifi-nar-bundles/nifi-provenance-repository-bundle/nifi-persistent-provenance-repository/src/main/java/org/apache/nifi/provenance/PersistentProvenanceRepository.java:
##########
@@ -226,6 +229,8 @@ public PersistentProvenanceRepository(final NiFiProperties nifiProperties) throw
}
public PersistentProvenanceRepository(final RepositoryConfiguration configuration, final int rolloverCheckMillis) throws IOException {
+ deprecationLogger.warn("{} should be replaced with WriteAheadProvenanceRepository", getClass().getSimpleName());
Review Comment:
Should we extend the logging a bit to point users at the right place to make the needed modification? For example, this is a fix to `nifi.properties`, where the `ListHDFS` change should be made in the processor properties.
##########
nifi-docs/src/main/asciidoc/administration-guide.adoc:
##########
@@ -182,6 +182,41 @@ Antivirus software can take a long time to scan large directories and the numero
* `provenance_repository`
* `state`
+[[logging_configuration]]
+== Logging Configuration
+NiFi uses link:https://logback.qos.ch/[logback^] as the runtime logging implementation. The `conf` directory contains a
+standard `logback.xml` configuration with default appender and level settings. The
+link:https://logback.qos.ch/manual/index.html[logback manual] provides a complete reference of available options.
+
+=== Standard Log Files
+The standard logback configuration includes the following appender definitions and associated log files:
+
+[options="header"]
+|=========================
+| File | Description
+| `nifi-app.log` | Application log containing framework and component messages
+| `nifi-bootstrap.log` | Bootstrap log containing startup and shutdown messages
+| `nifi-deprecation.log` | Deprecation log containing warnings for deprecated components and features
+| `nifi-request.log` | HTTP request log containing user interface and REST API access messages
+| `nifi-user.log` | User log containing authentication and authorization messages
+|=========================
+
+=== Deprecation Logging
+The `nifi-deprecation.log` contains warning messages describing components and features that will be removed in
+subsequent versions. Deprecation warnings should be evaluated and addressed to avoid breaking changes when upgrading to
+a new major version. Resolving deprecation warnings involves upgrading to new components, changing component property
+settings, or refactoring custom component classes.
+
Review Comment:
Some clarifying verbiage here might be helpful:
(first draft)
Before upgrading to a new version of NiFi, it should be a checklist item to run NiFi for $DURATION with an empty `nifi-deprecation.log`. If the upgrade spans multiple NiFi versions, the upgrade checklist should consider the deprecation notices in each relevant release notes document. Before a major version upgrade, it is suggested that an intervening upgrade be performed to the latest minor release, in order to evaluate the flow deprecation log for needed adjustments.
##########
nifi-nar-bundles/nifi-hadoop-bundle/nifi-hdfs-processors/src/main/java/org/apache/nifi/processors/hadoop/ListHDFS.java:
##########
@@ -277,6 +281,14 @@ public Set<Relationship> getRelationships() {
@Override
protected Collection<ValidationResult> customValidate(ValidationContext context) {
+ if (context.getProperty(DISTRIBUTED_CACHE_SERVICE).isSet()) {
Review Comment:
It looks like disabling a processor with this style of deprecation logging causes the logging to stop. Not sure where, but that might be a good thing to note... somewhere.
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org