Application usage statistics #2387

alex-odysseus · 2024-08-27T18:31:51Z

Addressing #2329

Business logic is based on parsing Audit Trail log entries. As some of the Analysis Execution entries were identified as duplicates a filter has been introduced to count each Analysis Execution precisely once

Usage examples (for the second one {} is used in the 'urlPattern' parameter to indicated a dynamic part of the URL):

curl --location --request POST 'localhost:8080/WebAPI/statistic/executions' \
--header 'Content-Type: application/json' \
--header 'Cookie: JSESSIONID=398******************EB0' \
--data-raw '{
"startDate":"2023-11-01",
"endDate":"2023-11-10",
"sourceKey":"SYNPUF_110k"
}'

curl --location --request POST 'localhost:8080/WebAPI/statistic/accesstrends' \
--header 'Content-Type: application/json' \
--header 'Cookie: JSESSIONID=398************************EB0' \
--data-raw '{
"startDate":"2024-03-12",
"endDate":"2024-03-12",
"endpoints":[{"method":"POST","urlPattern":"/WebAPI/conceptset/check"},{"method":"GET","urlPattern":"/WebAPI/i18n/locales"},{"method":"GET","urlPattern":"/WebAPI/cdmresults/{}/dashboard"}],
"responseFormat":"CSV",
"showUserInformation":true
}'

alex-odysseus · 2024-08-27T18:47:03Z

src/main/java/org/ohdsi/webapi/statistic/controller/StatisticController.java

+
+@Controller
+@Path("/statistic/")
+@ConditionalOnProperty(value = "audit.trail.enabled", havingValue = "true")


If Audit Trail feature is not enabled the Controller is not efficient

Is the intent here to make the /statistic/ end-point available based on the audit.trail.enabled property? Seems a little confusing to make an endpoint conditional vs. having it always available and to have it return based on the status of the configuration of the application.

I personally see no problem that some of the endpoints are not available if an appropriate configuration is not at place

I agree with @anthonysena, it is confusing because people will look at api docs and think they can call an endpoint, but it won't exist, which is confusing.

I don't see any documentation that describes that this end-piont doesn't exist when the configuration is not set.

We have rules about opt-in flags for default behavior (such as logging should be turned on/off, or auditing is enabled/distables) but the invocation of a end-point is the opt-in request of the user. If it's not enabled, it would be better to raise an error message saying 'auditing is not enabled' then return a 404 that a conditional-endpoint does not exist.

Changed as suggested

alex-odysseus · 2024-08-27T18:47:29Z

src/main/java/org/ohdsi/webapi/statistic/controller/StatisticController.java

+    private StatisticService service;
+
+    public enum ResponseFormat {
+        CSV, JSON


Both CSV and JSON output formats should be supported

src/main/java/org/ohdsi/webapi/statistic/controller/StatisticController.java

alex-odysseus · 2024-08-27T18:54:25Z

src/main/java/org/ohdsi/webapi/statistic/controller/StatisticController.java

+        }
+    }
+
+    private void updateExecutionStatisticsHeader(boolean showUserInformation) {


Both methods to be replaced by extra two static constants!

When will this be replaced?

alex-odysseus · 2024-08-27T18:55:57Z

src/main/java/org/ohdsi/webapi/statistic/dto/SourceExecutionDto.java

+    private String sourceName;
+    private String executionName;
+    private String executionDate;
+    private String userID;


When do you plan to change this?

alex-odysseus · 2024-08-27T19:11:20Z

src/main/java/org/ohdsi/webapi/statistic/service/StatisticService.java

+    protected final Logger LOG = LoggerFactory.getLogger(getClass());
+
+    @Value("${audit.trail.log.file}")
+    // TODO remove value


To be configured via properties

So will you remove this then?

alex-odysseus · 2024-08-27T19:20:42Z

...migration/postgresql/V2.14.0.20231206190600__fixing_statistics_enpoint_permission_method.sql

@@ -0,0 +1,2 @@
+UPDATE ${ohdsiSchema}.sec_permission SET value='statistic:executions:post' 


We may combine both scripts into one

Yes, please do and make sure the prefix of the file is v2.15...

anthonysena · 2024-12-27T17:45:21Z

src/main/java/org/ohdsi/webapi/statistic/controller/StatisticController.java

+
+@Controller
+@Path("/statistic/")
+@ConditionalOnProperty(value = "audit.trail.enabled", havingValue = "true")


Is the intent here to make the /statistic/ end-point available based on the audit.trail.enabled property? Seems a little confusing to make an endpoint conditional vs. having it always available and to have it return based on the status of the configuration of the application.

src/main/java/org/ohdsi/webapi/statistic/controller/StatisticController.java

anthonysena · 2024-12-27T17:47:42Z

src/main/java/org/ohdsi/webapi/statistic/controller/StatisticController.java

+        }
+    }
+
+    private void updateExecutionStatisticsHeader(boolean showUserInformation) {


When will this be replaced?

anthonysena · 2024-12-27T17:47:56Z

src/main/java/org/ohdsi/webapi/statistic/dto/SourceExecutionDto.java

+    private String sourceName;
+    private String executionName;
+    private String executionDate;
+    private String userID;


When do you plan to change this?

anthonysena · 2024-12-27T17:48:12Z

src/main/java/org/ohdsi/webapi/statistic/service/StatisticService.java

+    protected final Logger LOG = LoggerFactory.getLogger(getClass());
+
+    @Value("${audit.trail.log.file}")
+    // TODO remove value


So will you remove this then?

anthonysena · 2024-12-27T17:48:29Z

...migration/postgresql/V2.14.0.20231206190600__fixing_statistics_enpoint_permission_method.sql

@@ -0,0 +1,2 @@
+UPDATE ${ohdsiSchema}.sec_permission SET value='statistic:executions:post' 


Yes, please do and make sure the prefix of the file is v2.15...

chrisknoll · 2025-02-25T17:25:40Z

Docker build fails due to (presumably) a JDK 11 docker config. We discussed and possible least-friction solution is to check the stream syntax on regex Matcher in the stream API. Failing that, may need to find an ARM compatible JDK 8 for docker.

…der collections are stateless, hard-coded constants deleted relying on the application configuration file Migration scripts combined

oleg-odysseus · 2025-02-27T18:55:21Z

fixed issue with an invalid method reference exception during docker build, rebased.

…with a 404 Not Found instead when the Audit Trail property is not switched on

chrisknoll · 2025-03-11T13:25:30Z

src/main/java/org/ohdsi/webapi/statistic/controller/StatisticController.java

@@ -92,6 +98,9 @@ public Response executionStatistics(ExecutionStatisticsRequest executionStatisti
    @Produces(MediaType.APPLICATION_JSON)
    @Consumes(MediaType.APPLICATION_JSON)
    public Response accessStatistics(AccessTrendsStatisticsRequest accessTrendsStatisticsRequest) {
+    	if (!auditTrailEnabled) {
+    		throw new NotFoundException("Audit Trail functionality should be enabled (audit.trail.enabled) to serve this endpoint");


NotFound results in a 404, correct? We said that it would be confusing if someone looked at an api and attempted to call an endpoint and it was not found. I think we mentioned raising an error, such that it could return an HTTP 500 or some other HTTP code that represents an error other than 'not found'?

I misread your passage, corrected to the internal server error

alex-odysseus requested review from chrisknoll and anthonysena August 27, 2024 18:31

alex-odysseus commented Aug 27, 2024

View reviewed changes

src/main/java/org/ohdsi/webapi/statistic/controller/StatisticController.java Show resolved Hide resolved

alex-odysseus commented Aug 27, 2024

View reviewed changes

anthonysena linked an issue Sep 3, 2024 that may be closed by this pull request

Application usage statistics #2329

Open

anthonysena assigned anthonysena and alex-odysseus and unassigned anthonysena Dec 17, 2024

anthonysena requested changes Dec 27, 2024

View reviewed changes

alex-odysseus assigned anthonysena and unassigned alex-odysseus Jan 8, 2025

ssuvorov-fls and others added 11 commits February 27, 2025 19:50

add execution and trends statistics

3981341

Feature readiness

05e64a5

Fixing statistics endpoint permission method GET -> POST

53e27a6

ATL-8: Atlas Usage Statistics

8e28b98

ATL-8: Addressing userId info

1fe7db0

ATL-8: Updating username logic

462d81d

ATL-8: Adding accesstrends regexp update

01c0d51

ATL-8: Adding userInformation to the response

af3f682

Minor formatting

4fe2f04

Log entry added, non-closed resources are correctly handled, file hea…

3233ba3

…der collections are stateless, hard-coded constants deleted relying on the application configuration file Migration scripts combined

Fixed issue with a method reference in a stream during the docker build

1a431c3

oleg-odysseus force-pushed the application-usage-statistics branch from 831e3e7 to 1a431c3 Compare February 27, 2025 18:52

Having the controller instantiated without conditions and responding …

038a60b

…with a 404 Not Found instead when the Audit Trail property is not switched on

alex-odysseus assigned chrisknoll Mar 11, 2025

alex-odysseus added 2 commits March 11, 2025 11:50

Removing a condition from the service as well

32c98a7

Correcting StatisticService instantiation

24ebe0c

chrisknoll reviewed Mar 11, 2025

View reviewed changes

500 Internal Server Error instead of 404 Not Found

82a3ac9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Application usage statistics #2387

Application usage statistics #2387

alex-odysseus commented Aug 27, 2024 •

edited

Loading

alex-odysseus Aug 27, 2024

anthonysena Dec 27, 2024

alex-odysseus Jan 8, 2025

chrisknoll Mar 4, 2025

alex-odysseus Mar 11, 2025

alex-odysseus Aug 27, 2024

alex-odysseus Aug 27, 2024

anthonysena Dec 27, 2024

alex-odysseus Aug 27, 2024

anthonysena Dec 27, 2024

alex-odysseus Aug 27, 2024

anthonysena Dec 27, 2024

alex-odysseus Aug 27, 2024

anthonysena Dec 27, 2024

anthonysena Dec 27, 2024

anthonysena Dec 27, 2024

anthonysena Dec 27, 2024

anthonysena Dec 27, 2024

anthonysena Dec 27, 2024

chrisknoll commented Feb 25, 2025

oleg-odysseus commented Feb 27, 2025

chrisknoll Mar 11, 2025

alex-odysseus Mar 11, 2025

		@@ -0,0 +1,2 @@
		UPDATE ${ohdsiSchema}.sec_permission SET value='statistic:executions:post'

Application usage statistics #2387

Are you sure you want to change the base?

Application usage statistics #2387

Conversation

alex-odysseus commented Aug 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrisknoll commented Feb 25, 2025

oleg-odysseus commented Feb 27, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alex-odysseus commented Aug 27, 2024 •

edited

Loading