Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

remove(core): remove easy telemetry #4486

Open
wants to merge 44 commits into
base: master
Choose a base branch
from
Open

remove(core): remove easy telemetry #4486

wants to merge 44 commits into from
+614 −4,403

Conversation

yanavasileva
Copy link
Member

@yanavasileva
remove(core): remove easy telemetry
ee94e98 
* remove telemetry endpoint property
@yanavasileva
* remove configuration properties
2c80305 
* remove database property
* remove database lock
* remove `connect` dependency
* remove unused logging
* empty out commands: `IsTelemetryEnabledCmd`
  * remove related command checks
  * `IsTelemetryEnabledCmd` returns always `false`, add javadoc
@yanavasileva
* adjust tests
4db10c2
@yanavasileva
* adjust quarkus tests
aa89fff
@yanavasileva
* remove TelemetryConnectPluginTest IT
89257a8
@yanavasileva
* remove connect process engine plugin
6176e59
@yanavasileva yanavasileva added ci:e2e Runs the frontend end-to-end tests. ci:all-as Runs the builds for all application servers. ci:default-build Runs the builds that have no explicit trigger (e.g. different history levels). ci:migration Runs the process instance migration builds. ci:no-build Prevents any CI stage from running. ci:rest-api Runs extra builds for the REST API (currently only WLS compatibility builds). ci:rolling-update Runs the rolling update builds. ci:spring-boot Runs the integration tests for the Spring Boot starter. ci:webapp-integration Runs the webapp integration builds. labels Jul 8, 2024
@yanavasileva yanavasileva self-assigned this Jul 8, 2024
@yanavasileva yanavasileva mentioned this pull request Jul 8, 2024
Open
18 tasks
@yanavasileva yanavasileva removed the ci:no-build Prevents any CI stage from running. label Jul 8, 2024
@yanavasileva yanavasileva removed the ci:spring-boot Runs the integration tests for the Spring Boot starter. label Jul 10, 2024
@yanavasileva
clean up TODO
0317b3e
@yanavasileva
adjust OpenAPI
422326e
@yanavasileva
fix rest api mock
7581400
@yanavasileva
remove TelemetryRegistry#telemetryLocallyActivated
746813f 
remove TelemetryUtil#toggleLocalTelemetry
@yanavasileva
telemetry sending task
26445e8 
*remove timer & cleanup #updateAndSendData
* rename to DiagnosticsCollector
@yanavasileva yanavasileva added the ci:default-build Runs the builds that have no explicit trigger (e.g. different history levels). label Jul 12, 2024
@yanavasileva yanavasileva added ci:e2e Runs the frontend end-to-end tests. ci:all-as Runs the builds for all application servers. ci:migration Runs the process instance migration builds. ci:rolling-update Runs the rolling update builds. ci:webapp-integration Runs the webapp integration builds. labels Jul 12, 2024
@yanavasileva
renaming telemetry -> diagnostics
d052960 
TelemetryRegistry -> DiagnosticsRegistry
PlatformTelemetryRegistry -> PlatformDiagnosticsRegistry
CommandChecker.checkReadDiagnosticsData ->
CommandChecker.checkReadTelemetryData
DeleteLicenseKeyCmd.updateTelemetry ->
DeleteLicenseKeyCmd.updateDiagnostics
ManagementServiceImpl.getLicenseKeyFromTelemetry() ->
ManagementServiceImpl.getLicenseKeyFromDiagnostics()

remove

* Telemetry logger
* ManagementServiceImpl.clearTelemetryData()
@yanavasileva
move
a370fe1 
org.camunda.bpm.engine.impl.telemetry.reporter.DiagnosticsCollector ->
org.camunda.bpm.engine.impl.telemetry.DiagnosticsCollector

org.camunda.bpm.engine.impl.telemetry.JavaClases ->
org.camunda.bpm.engine.impl.diagnostics
@yanavasileva
remove telemetry in distro/run and spring-boot-starter
74ea9e7
@yanavasileva
fix imports
2bbe663
@yanavasileva yanavasileva marked this pull request as ready for review July 16, 2024 15:00
joaquinfelici
joaquinfelici reviewed Jul 18, 2024
View reviewed changes
Copy link
Contributor

@joaquinfelici joaquinfelici left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Added a few comments.

engine/src/main/java/org/camunda/bpm/engine/impl/cfg/ProcessEngineConfigurationImpl.java
/** default: once every 24 hours */
protected long telemetryReportingPeriod = 24 * 60 * 60;
// diagnostics ///////////////////////////////////////////////////////
protected DiagnosticsCollector diagnosticsCollector;
protected TelemetryDataImpl telemetryData;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ Only reason we don't modify the name of this class to something related to diagnostics is because the interface is part of the public API?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I left the interface part of the public API and the implementation there untouched.

engine/src/main/java/org/camunda/bpm/engine/impl/cmd/CommandLogger.java Outdated
@@ -362,4 +362,9 @@ public ProcessEngineException exceptionJobRetriesMustNotBeNegative(Integer retri
"The number of job retries must be a non-negative Integer, but '{}' has been provided.", retries));
}

public ProcessEngineException exceptionWhileRetrievingDiagnosticsDataRegistryNull() {
return new ProcessEngineException(
exceptionMessage("019", "Error while retrieving diagnostics data. Diagnostics registry was not initialized."));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔧 Looks like id "019" is already taken above?

engine/src/main/java/org/camunda/bpm/engine/impl/diagnostics/DiagnosticsCollector.java Outdated
Comment on lines 41 to 48
protected static final Set<String> METRICS_TO_REPORT = new HashSet<>();

static {
METRICS_TO_REPORT.add(ROOT_PROCESS_INSTANCE_START);
METRICS_TO_REPORT.add(EXECUTED_DECISION_INSTANCES);
METRICS_TO_REPORT.add(EXECUTED_DECISION_ELEMENTS);
METRICS_TO_REPORT.add(ACTIVTY_INSTANCE_START);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙃 We can also use Set.of() to avoid the static block.

engine/src/main/java/org/camunda/bpm/engine/impl/util/ProcessEngineDetails.java Outdated
@@ -18,7 +18,7 @@

/**
* Holds process engine version and edition (enterprise or community)
* Used in retrieving the process engine details when sending telemetry data
* Used in retrieving the process engine details for diagnostic data
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Used in retrieving the process engine details for diagnostic data
* Used in retrieving the process engine details for diagnostics data

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joaquinfelici joaquinfelici joaquinfelici left review comments

Assignees

@yanavasileva yanavasileva

Labels
ci:all-as Runs the builds for all application servers. ci:default-build Runs the builds that have no explicit trigger (e.g. different history levels). ci:e2e Runs the frontend end-to-end tests. ci:migration Runs the process instance migration builds. ci:rest-api Runs extra builds for the REST API (currently only WLS compatibility builds). ci:rolling-update Runs the rolling update builds. ci:webapp-integration Runs the webapp integration builds.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@yanavasileva @joaquinfelici