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.

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

Analytics improvement #12176

Closed
wants to merge 11 commits into from
Closed

Analytics improvement #12176

wants to merge 11 commits into from

Conversation

BLasan
Copy link
Contributor

@BLasan BLasan commented Oct 24, 2023

Purpose

Goals

Load API and Operation Policies of the APIs/API Products to the gateway and send the corresponding policies in the analytics event.

Approach

  1. Load API Policies and Operation Policies to the gateway on the deployment.
  2. Fetch Operation and API Policies related to the the resource and the method on the API invocation.
  3. To Enable/Disable Policies in the event, a separate configuration enablePolicy is introduced under analytics configuration in deployment.toml file. If this is enabled then operation and API policies will get loaded to the analytics event
[apim.analytics]
enable = true
auth_token = ""
enablePolicy = true

@BLasan BLasan force-pushed the analytics-improvement branch 5 times, most recently from 587c32b to 7524f90 Compare October 27, 2023 14:58
@@ -0,0 +1,70 @@
/*
* Copyright (c) 2020, WSO2 Inc. (http://www.wso2.org) All Rights Reserved.
Copy link
Member

Choose a reason for hiding this comment

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

Let's change the license header

@@ -0,0 +1,65 @@
package org.wso2.carbon.apimgt.common.analytics.publishers.dto;
Copy link
Member

Choose a reason for hiding this comment

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

Let's include the license header for this file.

@@ -151,6 +158,41 @@ public API getApi() throws DataNotFoundException {
api.setApiVersion(apiObj.getApiVersion());
api.setApiCreator(apiObj.getApiProvider());
api.setApiCreatorTenantDomain(MultitenantUtils.getTenantDomain(api.getApiCreator()));
List<URITemplate> uriTemplates = new ArrayList<>();
for (URLMapping uriTemplate : apiObj.getUrlMappings()) {
org.wso2.carbon.apimgt.common.analytics.publishers.dto.URITemplate uriTemplateObj = new org.wso2.carbon.apimgt.common.analytics.publishers.dto.URITemplate();
Copy link
Member

Choose a reason for hiding this comment

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

Formatting has gone beyond 120 limit.

uriTemplateObj.setAuthScheme(uriTemplate.getAuthScheme());
List<org.wso2.carbon.apimgt.common.analytics.publishers.dto.OperationPolicy> operationPolicies = new ArrayList<>();
for (OperationPolicy operationPolicy : uriTemplate.getOperationPolicies()) {
org.wso2.carbon.apimgt.common.analytics.publishers.dto.OperationPolicy operationPolicyObj = new org.wso2.carbon.apimgt.common.analytics.publishers.dto.OperationPolicy();
Copy link
Member

Choose a reason for hiding this comment

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

Let's format the code.

}
List<org.wso2.carbon.apimgt.common.analytics.publishers.dto.OperationPolicy> apiPolicyList = new ArrayList<>();
for(OperationPolicy apiPolicy: apiObj.getApiPolicies()){
org.wso2.carbon.apimgt.common.analytics.publishers.dto.OperationPolicy operationPolicyObj = new org.wso2.carbon.apimgt.common.analytics.publishers.dto.OperationPolicy();
Copy link
Member

Choose a reason for hiding this comment

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

Let's format the code.


}

private void attachPolicies(Connection connection, String revisionId, API api) throws SQLException {
Copy link
Member

Choose a reason for hiding this comment

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

Let's fix the rest of the formatting issues in this file.

@@ -570,7 +570,26 @@ public class SubscriptionValidationSQLConstants {
"AM_API_RESOURCE_SCOPE_MAPPING.SCOPE_NAME FROM AM_API_URL_MAPPING LEFT JOIN AM_API_RESOURCE_SCOPE_MAPPING" +
" ON AM_API_URL_MAPPING.URL_MAPPING_ID=AM_API_RESOURCE_SCOPE_MAPPING.URL_MAPPING_ID WHERE " +
"AM_API_URL_MAPPING.API_ID = ? AND AM_API_URL_MAPPING.REVISION_UUID = ?";

// public static final String GET_OPERATION_POLICIES_PER_URI_BY_API_SQL = "SELECT AUM.HTTP_METHOD, AUM.URL_PATTERN, "
Copy link
Member

Choose a reason for hiding this comment

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

Let's remove the commented out code if not required.

@@ -0,0 +1,157 @@
package org.wso2.carbon.apimgt.internal.service.dto;
Copy link
Member

Choose a reason for hiding this comment

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

Let's add the license header and remove wildcard imports.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a swagger generated file. Should we modify this class as well?

Copy link
Member

Choose a reason for hiding this comment

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

Yes that's right, no need to add the header here as it is an auto generated file.

Copy link

codecov bot commented Nov 15, 2023

Codecov Report

Attention: 191 lines in your changes are missing coverage. Please review.

Comparison is base (0457739) 39.36% compared to head (2a61dfa) 20.83%.
Report is 152 commits behind head on master.

Files Patch % Lines
...bon/apimgt/impl/dao/SubscriptionValidationDAO.java 0.00% 90 Missing ⚠️
.../service/utils/SubscriptionValidationDataUtil.java 0.00% 48 Missing ⚠️
...ndlers/analytics/SynapseAnalyticsDataProvider.java 0.00% 36 Missing ⚠️
.../wso2/carbon/apimgt/api/model/OperationPolicy.java 0.00% 4 Missing ⚠️
...wso2/carbon/apimgt/api/model/subscription/API.java 0.00% 4 Missing ⚠️
...rbon/apimgt/api/model/subscription/URLMapping.java 0.00% 4 Missing ⚠️
...rg/wso2/carbon/apimgt/keymgt/model/entity/API.java 25.00% 3 Missing ⚠️
...o2/carbon/apimgt/impl/APIManagerConfiguration.java 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #12176       +/-   ##
===========================================
- Coverage   39.36%   20.83%   -18.53%     
===========================================
  Files        1789     1193      -596     
  Lines      131570   101689    -29881     
  Branches    19008    14280     -4728     
===========================================
- Hits        51797    21191    -30606     
- Misses      73195    77710     +4515     
+ Partials     6578     2788     -3790     
Flag Coverage Δ
integration_tests ?
unit_tests 20.83% <0.52%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

org.wso2.carbon.apimgt.common.analytics.publishers.dto.URITemplate uriTemplateObj
= new org.wso2.carbon.apimgt.common.analytics.publishers.dto.URITemplate();
if (uriTemplate.getHttpMethod() != null && uriTemplate.getHttpMethod()
.equals(messageContext.getProperty("api.ut.HTTP_METHOD")) && uriTemplate.getUrlPattern() != null
Copy link
Member

Choose a reason for hiding this comment

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

@@ -100,8 +102,32 @@ private static APIDTO fromAPItoDTO(API model) {
urlMappingDTO.setThrottlingPolicy(urlMapping.getThrottlingPolicy());
urlMappingDTO.setUrlPattern(urlMapping.getUrlPattern());
urlMappingDTO.setScopes(urlMapping.getScopes());
List<OperationPolicyDTO> operationPolicyDTOList = new ArrayList<>();
for(OperationPolicy operationPolicy: urlMapping.getOperationPolicies()) {
Copy link
Member

Choose a reason for hiding this comment

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

Formatting

urlMappingsDTO.add(urlMappingDTO);
}
List<OperationPolicyDTO> apiPolicies = new ArrayList<>();
for(OperationPolicy apiPolicy: model.getApiPolicies()) {
Copy link
Member

Choose a reason for hiding this comment

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

Formatting

@@ -132,8 +158,32 @@ public static APIListDTO fromAPIToAPIListDTO(API model) {
urlMappingDTO.setThrottlingPolicy(urlMapping.getThrottlingPolicy());
urlMappingDTO.setUrlPattern(urlMapping.getUrlPattern());
urlMappingDTO.setScopes(urlMapping.getScopes());
List<OperationPolicyDTO> operationPolicyDTOList = new ArrayList<>();
for(OperationPolicy operationPolicy: urlMapping.getOperationPolicies()) {
Copy link
Member

Choose a reason for hiding this comment

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

Formatting

urlMappingsDTO.add(urlMappingDTO);
}
List<OperationPolicyDTO> apiPolicies = new ArrayList<>();
for(OperationPolicy apiPolicy: model.getApiPolicies()) {
Copy link
Member

Choose a reason for hiding this comment

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

Formatting issue. You can use https://tharika.medium.com/how-to-setup-code-formatting-in-my-ide-86374ec8fbd2 to fix the formatting using a template instead of requiring to manually do it.

tharikaGitHub
tharikaGitHub previously approved these changes Dec 21, 2023
@@ -1170,10 +1177,16 @@ private void attachURlMappingDetailsOfApiProduct(Connection connection, API api)
}
}
}

if(configs.containsKey(POLICY_ENABLED_FOR_ANALYTICS)) {
Copy link
Member

Choose a reason for hiding this comment

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

Formatting

@@ -1260,6 +1273,112 @@ private void attachURLMappingDetails(Connection connection, String revisionId, A
}
}
}

if(configs.containsKey(POLICY_ENABLED_FOR_ANALYTICS)) {
Copy link
Member

Choose a reason for hiding this comment

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

Formatting

@BLasan
Copy link
Contributor Author

BLasan commented Jan 19, 2024

Replication of #12207

@BLasan BLasan closed this Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Is Api using mediation policies
2 participants