OS Core Lib Azure merge requestshttps://community.opengroup.org/osdu/platform/system/lib/cloud/azure/os-core-lib-azure/-/merge_requests2023-08-18T12:44:57Zhttps://community.opengroup.org/osdu/platform/system/lib/cloud/azure/os-core-lib-azure/-/merge_requests/93Managing common properties from core-lib-azure2023-08-18T12:44:57ZAbhishek PatilManaging common properties from core-lib-azure## All Submissions:
-------------------------------------
* [YES] I have added an explanation of what changes in this merge do and why we should include it?
* [YES] I have updated the documentation accordingly.
* [NA] I have added tests ...## All Submissions:
-------------------------------------
* [YES] I have added an explanation of what changes in this merge do and why we should include it?
* [YES] I have updated the documentation accordingly.
* [NA] I have added tests to cover my changes.
* [YES] All new and existing tests passed.
* [YES] My code follows the code style of this project.
* [NA] I ran lint checks locally prior to submission.
## What is the issue or story related to the change?
-------------------------------------
<!-- Please describe the current behavior that you are modifying, 'or' link to a relevant issue.
Feel free to add references to any design documents you might have shared with the team or any
related MR that you are building on top of. -->
There are various configuration properties which are common across all OSDU services. Any update/addition to these common properties require change in each and every service using it.
High level design:
https://microsoft.sharepoint.com/:w:/t/EnergyDataPlatform/EckjpjanywtMhRIzwgHotXABpRuHueQl1GSmdySPn0tNnw?e=XfZIlw
Issue: <!-- Link any __GitLab__ workitem(s) to this pull request. -->
https://dev.azure.com/msazure/One/_sprints/taskboard/Azure%20Global%20Engineering%20Energy%20Team/One/AGE/Cobalt/May?workitem=9872385
<!-- Please add implementation details of current set of changes and how the code changes are
doing what they are expected to do. Are there any complex loops or designated code blocks that
should be elaborated? Is there some contextual knowledge that the reviewer should be aware of? -->
Change details:
Adding common.properties file which will contain all the configuration properties which are common across all OSDU services. Services will use this common.properties along with their own application.properties.
## Test coverage:
------------------
<!-- Mention unit test coverage of changes. -->
## Does this introduce a breaking change?
-------------------------------------
- [NO]
<!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. -->
## Pending items
----------------
<!-- Are there changes that you'll introduce in upcoming MRs and hence did not add in this one? Next steps of your
feature can also be mentioned here. -->
## Reviewer request
-------------------
- Please provide an ETA when you plan to review this MR. Write a comment to decline or provide an ETA.
- Block the MR if you feel there is less testing or no details in the MR
- Please cover the following aspects in the MR
-- Coding design: _\<Reviewer1>_
-- Backward Compatibility: _\<Reviewer2>_
-- Feature Logic: _\<Logic design\>_
-- _\<Any other context mention here>_
OR
-- _\<Component 1>_: _\<Reviewer1>_
-- _\<CosmosDB>_: _\<Reviewer2>_
-- _\<ServiceBus>_ _\<Reviewer3>_
-- _\<Mention any other component and owner>_
## Other information
-------------------------------------
<!-- Any other information that is important to this MR such as screenshots of how the component looks before and after the change. -->M7 - Release 0.10Abhishek PatilAbhishek Patilhttps://community.opengroup.org/osdu/platform/system/lib/cloud/azure/os-core-lib-azure/-/merge_requests/95Adding the maven release-candidiate script to this library's pipeline2023-08-18T12:44:55ZDavid Diederichd.diederich@opengroup.orgAdding the maven release-candidiate script to this library's pipelineM6 - Release 0.9David Diederichd.diederich@opengroup.orgDavid Diederichd.diederich@opengroup.orghttps://community.opengroup.org/osdu/platform/system/lib/cloud/azure/os-core-lib-azure/-/merge_requests/96add ignored servlet paths for Transaction Log Filter;2023-08-18T12:44:53ZYauheni Lesnikauadd ignored servlet paths for Transaction Log Filter;For the purpose of the logging optimization was decided to introduce the mechanism to not log some calls (i.e. /actuator/health) into _org.opengroup.osdu.azure.filters.TransactionLogFilter_For the purpose of the logging optimization was decided to introduce the mechanism to not log some calls (i.e. /actuator/health) into _org.opengroup.osdu.azure.filters.TransactionLogFilter_M6 - Release 0.9ethiraj krishnamanaiduNitin-slbNeelesh ThakurDaniel SchollImran SiddiqueAlok Joshiashley kelhamViacheslav Tarasov - SLBYauheni LesnikauSanjeev-SLBTika Lestari [SLB]Madhur Tanwani [Microsoft]ethiraj krishnamanaiduhttps://community.opengroup.org/osdu/platform/system/lib/cloud/azure/os-core-lib-azure/-/merge_requests/98Added Configurable retries to Cosmos2023-08-18T12:44:49ZRonak SakhujaAdded Configurable retries to Cosmos## All Submissions:
-------------------------------------
* [YES] I have added an explanation of what changes in this merge do and why we should include it?
* [YES] I have updated the documentation accordingly.
* [YES] I have added tests...## All Submissions:
-------------------------------------
* [YES] I have added an explanation of what changes in this merge do and why we should include it?
* [YES] I have updated the documentation accordingly.
* [YES] I have added tests to cover my changes.
* [YES] All new and existing tests passed.
* [YES] My code follows the code style of this project.
* [YES] I ran lint checks locally prior to submission.
## What is the issue or story related to the change?
-------------------------------------
CosmosDB offers option for configuring MaxRetryAttempts and MaxRetryWaitTime. For any service who wish not to use the default values, and want to provide custom values for the mentioned attributes, they will be able to after this change.
High level design:
![image](/uploads/24e32c7ff6c8541f98fed70324aa998b/image.png)
Issue: <!-- Link any __GitLab__ workitem(s) to this pull request. -->
We are adding some configurable options for retries in CosmosDB to common.properties. Services can override these properties through application.properties and set their own custom values.
Sample values
azure.cosmos.maxRetryCount=9
azure.cosmos.retryWaitTimeout=30
## Test coverage:
------------------
<!-- Mention unit test coverage of changes. -->
All changes are covered.
All ITs passed
![image](/uploads/44d9da1896144dba82d15f0eb6633354/image.png)
## Does this introduce a breaking change?
-------------------------------------
- [NO]
<!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. -->
## Pending items
----------------
<!-- Are there changes that you'll introduce in upcoming MRs and hence did not add in this one? Next steps of your
feature can also be mentioned here. -->
## Reviewer request
-------------------
- Please provide an ETA when you plan to review this MR. Write a comment to decline or provide an ETA.
- Block the MR if you feel there is less testing or no details in the MR
- Please cover the following aspects in the MR
-- Coding design: _\<Reviewer1>_
-- Backward Compatibility: _\<Reviewer2>_
-- Feature Logic: _\<Logic design\>_
-- _\<Any other context mention here>_
OR
-- _\<Component 1>_: _\<Reviewer1>_
-- _\<CosmosDB>_: _\<Reviewer2>_
-- _\<ServiceBus>_ _\<Reviewer3>_
-- _\<Mention any other component and owner>_
## Other information
-------------------------------------
<!-- Any other information that is important to this MR such as screenshots of how the component looks before and after the change. -->M7 - Release 0.10Ronak SakhujaRonak Sakhujahttps://community.opengroup.org/osdu/platform/system/lib/cloud/azure/os-core-lib-azure/-/merge_requests/100Updated azure-spring-data-cosmos dependency version to 3.7 to use the latest ...2023-08-18T12:44:47ZSmitha ManjunathUpdated azure-spring-data-cosmos dependency version to 3.7 to use the latest azure sdk (4.15)## All Submissions:
-------------------------------------
* [YES] I have added an explanation of what changes in this merge do and why we should include it?
* [NO] I have updated the documentation accordingly.
* [NA] I have added tests t...## All Submissions:
-------------------------------------
* [YES] I have added an explanation of what changes in this merge do and why we should include it?
* [NO] I have updated the documentation accordingly.
* [NA] I have added tests to cover my changes.
* [YES/NO/NA] All new and existing tests passed.
* [YES/NO/NA] My code follows the code style of this project.
* [YES/NO/NA] I ran lint checks locally prior to submission.
## What is the issue or story related to the change?
-------------------------------------
<!-- Please describe the current behavior that you are modifying, 'or' link to a relevant issue.
Feel free to add references to any design documents you might have shared with the team or any
related MR that you are building on top of. -->
Regarding issue : https://dev.azure.com/msazure/One/_sprints/taskboard/Azure%20Global%20Engineering%20Energy%20Team/One/AGE/Cobalt/May?workitem=9911670
High level design:
Issue: <!-- Link any __GitLab__ workitem(s) to this pull request. -->
<!-- Please add implementation details of current set of changes and how the code changes are
doing what they are expected to do. Are there any complex loops or designated code blocks that
should be elaborated? Is there some contextual knowledge that the reviewer should be aware of? -->
Change details:
## Test coverage:
------------------
<!-- Mention unit test coverage of changes. -->
## Does this introduce a breaking change?
-------------------------------------
- [YES/**NO**]
<!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. -->
## Pending items
----------------
<!-- Are there changes that you'll introduce in upcoming MRs and hence did not add in this one? Next steps of your
feature can also be mentioned here. -->
## Reviewer request
-------------------
- Please provide an ETA when you plan to review this MR. Write a comment to decline or provide an ETA.
- Block the MR if you feel there is less testing or no details in the MR
- Please cover the following aspects in the MR
-- Coding design: _\<Reviewer1>_
-- Backward Compatibility: _\<Reviewer2>_
-- Feature Logic: _\<Logic design\>_
-- _\<Any other context mention here>_
OR
-- _\<Component 1>_: _\<Reviewer1>_
-- _\<CosmosDB>_: _\<Reviewer2>_
-- _\<ServiceBus>_ _\<Reviewer3>_
-- _\<Mention any other component and owner>_
## Other information
-------------------------------------
<!-- Any other information that is important to this MR such as screenshots of how the component looks before and after the change. -->M6 - Release 0.9Smitha ManjunathSmitha Manjunathhttps://community.opengroup.org/osdu/platform/system/lib/cloud/azure/os-core-lib-azure/-/merge_requests/101Upgrade os-core-common library2023-08-18T12:44:45ZRostislav Vatolinvatolinrp@gmail.comUpgrade os-core-common libraryUpgraded library os-core-common to 0.9.0-rc13 so latest changes related to elasticsearch libraries are applied.
The latest version of os-core-common has ILogger interface updated: included debug methods. Having that, the new methods were...Upgraded library os-core-common to 0.9.0-rc13 so latest changes related to elasticsearch libraries are applied.
The latest version of os-core-common has ILogger interface updated: included debug methods. Having that, the new methods were implemented in Slf4JLogger class.
The new library was tested with the following service: crs-catalog-service: https://community.opengroup.org/osdu/platform/system/reference/crs-catalog-service/-/merge_requests/47M6 - Release 0.9https://community.opengroup.org/osdu/platform/system/lib/cloud/azure/os-core-lib-azure/-/merge_requests/104Blobstore Retry Configuration2023-08-18T12:44:43ZRonak SakhujaBlobstore Retry Configuration## All Submissions:
-------------------------------------
* [YES] I have added an explanation of what changes in this merge do and why we should include it?
* [NO] I have updated the documentation accordingly.
* [YES] I have added tests ...## All Submissions:
-------------------------------------
* [YES] I have added an explanation of what changes in this merge do and why we should include it?
* [NO] I have updated the documentation accordingly.
* [YES] I have added tests to cover my changes.
* [YES] All new and existing tests passed.
* [YES] My code follows the code style of this project.
* [YES] I ran lint checks locally prior to submission.
## What is the issue or story related to the change?
-------------------------------------
BlobStore offers option for configuring various attributes for retry policy. For any service who wish not to use the default values, and want to provide custom values for the mentioned attributes, they will be able to after this change.
High level design:
![image](/uploads/5d300661ada45b326a9c0043da4f9b89/image.png)
Issue: <!-- Link any __GitLab__ workitem(s) to this pull request. -->
Change details:
Variables that can be added to application.properties are:
azure.blobstore.maxTries
azure.blobstore.tryTimeoutInSeconds
azure.blobstore.retryDelayInMs
azure.blobstore.maxRetryDelayInMs
azure.blobstore.retryPolicyType
azure.blobstore.secondaryHost
## Test coverage:
------------------
All changes are covered.
![image](/uploads/93e03dc4635021ba219dbefad4cdbca1/image.png)
## Does this introduce a breaking change?
-------------------------------------
- NO
## Pending items
----------------
## Reviewer request
-------------------
- Please provide an ETA when you plan to review this MR. Write a comment to decline or provide an ETA.
- Block the MR if you feel there is less testing or no details in the MR
- Please cover the following aspects in the MR
-- Coding design: _\<Reviewer1>_
-- Backward Compatibility: _\<Reviewer2>_
-- Feature Logic: _\<Logic design\>_
-- _\<Any other context mention here>_
OR
-- _\<Component 1>_: _\<Reviewer1>_
-- _\<CosmosDB>_: _\<Reviewer2>_
-- _\<ServiceBus>_ _\<Reviewer3>_
-- _\<Mention any other component and owner>_
## Other information
-------------------------------------
<!-- Any other information that is important to this MR such as screenshots of how the component looks before and after the change. -->M7 - Release 0.10https://community.opengroup.org/osdu/platform/system/lib/cloud/azure/os-core-lib-azure/-/merge_requests/105Added Retry for eventgrid2023-08-18T12:44:41ZRonak SakhujaAdded Retry for eventgrid## All Submissions:
-------------------------------------
* [YES] I have added an explanation of what changes in this merge do and why we should include it?
* [YES] I have updated the documentation accordingly.
* [YES] I have added tests...## All Submissions:
-------------------------------------
* [YES] I have added an explanation of what changes in this merge do and why we should include it?
* [YES] I have updated the documentation accordingly.
* [YES] I have added tests to cover my changes.
* [YES] All new and existing tests passed.
* [YES] My code follows the code style of this project.
* [YES] I ran lint checks locally prior to submission.
## What is the issue or story related to the change?
-------------------------------------
Eventgrid offers option for configuring Timeout values. For any service who wish not to use the default values, and want to provide custom values for the mentioned attributes, they will be able to after this change.
High level design:
![image](/uploads/a8ddd03daadbb289f4d25077a9bbc5a2/image.png)
Issue: <!-- Link any __GitLab__ workitem(s) to this pull request. -->
<!-- Please add implementation details of current set of changes and how the code changes are
doing what they are expected to do. Are there any complex loops or designated code blocks that
should be elaborated? Is there some contextual knowledge that the reviewer should be aware of? -->
Change details:
longRunningOperationRetryTimeout can be configured using
azure.eventgridtopic.longRunningOperationRetryTimeout
## Test coverage:
------------------
All changes are covered.
## Does this introduce a breaking change?
-------------------------------------
- YES
<!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. -->
## Pending items
----------------
## Reviewer request
-------------------
- Please provide an ETA when you plan to review this MR. Write a comment to decline or provide an ETA.
- Block the MR if you feel there is less testing or no details in the MR
- Please cover the following aspects in the MR
-- Coding design: _\<Reviewer1>_
-- Backward Compatibility: _\<Reviewer2>_
-- Feature Logic: _\<Logic design\>_
-- _\<Any other context mention here>_
OR
-- _\<Component 1>_: _\<Reviewer1>_
-- _\<CosmosDB>_: _\<Reviewer2>_
-- _\<ServiceBus>_ _\<Reviewer3>_
-- _\<Mention any other component and owner>_
## Other information
-------------------------------------
<!-- Any other information that is important to this MR such as screenshots of how the component looks before and after the change. -->M7 - Release 0.10https://community.opengroup.org/osdu/platform/system/lib/cloud/azure/os-core-lib-azure/-/merge_requests/107Configurable Retry And Timeout for Entitlements Service2023-08-18T12:44:40ZMuskan SrivastavaConfigurable Retry And Timeout for Entitlements Service## All Submissions:
-------------------------------------
* I have added an explanation of what changes in this merge do and why we should include it? - YES
* Does the MR contain pipeline/ helm chart related changes?- NO
* I have updated...## All Submissions:
-------------------------------------
* I have added an explanation of what changes in this merge do and why we should include it? - YES
* Does the MR contain pipeline/ helm chart related changes?- NO
* I have updated the documentation accordingly. - YES
* I have added tests to cover my changes. -NA
* All new and existing tests passed. -YES
* My code follows the code style of this project. -YES
* I ran lint checks locally prior to submission. -YES
## What is the issue or story related to the change?
-------------------------------------
This change is a step forward to enable the services to configure following variables for service-to-service communication:
- Retry count for service unavailable strategy
- Connect timeout
- Connection Request Timeout
- Socket Timeout
These configurations can only be used for entitlements service for now.
High level design: This MR adds 5 classes in total. <br>
- RetryAndTimeoutConfiguration : contains configuration variables
- HttpClientHandlerAzure : extends HttpClientHandler, and sets variables as picked up by RetryAndTimeoutConfiguration
- HttpClientAzure : uses UrlFetchServiceImpl(which in turn use IHttpClientHandler type) internally, and implements IHttpClient. Currently, Services require a type of IHttpClient to make service-to-service calls
- EntitlementsAPIConfigAzure : creates a bean of type EntitlementsAPIConfig
- EntitlementsFactoryAzure : uses newly created HttpClientAzure internally so that configurable retires can be used for Entitlements Service.
## Does this introduce a breaking change?
-------------------------------------
- NO
<!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. -->
## Pending items
----------------
Following MRs will be brought further :
- Addition of Factory classes of other services on lines of EntitlementsFactoryAzure will be added.M7 - Release 0.10Muskan SrivastavaMuskan Srivastavahttps://community.opengroup.org/osdu/platform/system/lib/cloud/azure/os-core-lib-azure/-/merge_requests/109Fetch only the required event grid topic details2023-08-18T12:44:37ZAlok JoshiFetch only the required event grid topic detailsUpon Storage PUT api (a `recordschanged` event), we are trying to get all event grid properties from partition info, and successively their secret value from KV. This is redundant. As we add other topics for other events, we only want to...Upon Storage PUT api (a `recordschanged` event), we are trying to get all event grid properties from partition info, and successively their secret value from KV. This is redundant. As we add other topics for other events, we only want to pull the event grid properties for the relevant event.
For example, one of the PUTC workflows is adding schemachanged event. Currently, even this property gets pulled (since it matches the regex). This causes 2 issues:
1. If the secrets associated with this property are missing in KV, Storage PUT records workflow doesn't work. This is a major blocker for the most used api
2. This is also causing throttling issues on KV access due to too many reads
This change aims to fix this issue by accessing eventgrid property only relevant to topic nameM7 - Release 0.10Alok JoshiAlok Joshihttps://community.opengroup.org/osdu/platform/system/lib/cloud/azure/os-core-lib-azure/-/merge_requests/112Reduce info logs for core services2023-08-18T12:44:35ZAlok JoshiReduce info logs for core services## All Submissions:
-------------------------------------
* [YES] I have added an explanation of what changes in this merge do and why we should include it?
* [YES] I have updated the documentation accordingly.
* [NA] I have added tests ...## All Submissions:
-------------------------------------
* [YES] I have added an explanation of what changes in this merge do and why we should include it?
* [YES] I have updated the documentation accordingly.
* [NA] I have added tests to cover my changes.
* [YES] All new and existing tests passed.
* [YES] My code follows the code style of this project.
* [NA] I ran lint checks locally prior to submission.
## What is the issue or story related to the change?
-------------------------------------
This change is an effort to reduce info logs for core services. Combining all the AKS services we deploy in our prod, customer environments, we regularly see ContainerLog usage crossing 200 or 300gb daily. Since ApplicationInsight logging is not cheap, this is incurring huge costs.
With this change, we are reducing the number of trace logs that are otherwise not required for 'happy path' for a service request. Please see attached an example of Storage's POST /query/records api request log details from our dev env.
![trace_logs](/uploads/8f8f742ead5aa3291609d5a2f1ed358a/trace_logs.PNG)
The highlighted entries seem unnecessary and given the volume of requests, add up to quite a lot of size. This change aims to remove these entries to cost-optimize logging.
## Test coverage:
------------------
No change
## Does this introduce a breaking change?
-------------------------------------
No
## Pending items
----------------
<!-- Are there changes that you'll introduce in upcoming MRs and hence did not add in this one? Next steps of your
feature can also be mentioned here. -->
## Reviewer request
-------------------
- Please provide an ETA when you plan to review this MR. Write a comment to decline or provide an ETA.
- Block the MR if you feel there is less testing or no details in the MR
- Please cover the following aspects in the MR
-- Coding design: _\<Reviewer1>_
-- Backward Compatibility: _\<Reviewer2>_
-- Feature Logic: _\<Logic design\>_
-- _\<Any other context mention here>_
OR
-- _\<Component 1>_: _\<Reviewer1>_
-- _\<CosmosDB>_: _\<Reviewer2>_
-- _\<ServiceBus>_ _\<Reviewer3>_
-- _\<Mention any other component and owner>_
## Other information
-------------------------------------
<!-- Any other information that is important to this MR such as screenshots of how the component looks before and after the change. -->M7 - Release 0.10Alok JoshiAlok Joshihttps://community.opengroup.org/osdu/platform/system/lib/cloud/azure/os-core-lib-azure/-/merge_requests/113Enable logging for health check fails2023-08-18T12:44:33ZSmitha ManjunathEnable logging for health check fails## All Submissions:
-------------------------------------
* [YES/NO] I have added an explanation of what changes in this merge do and why we should include it?
* [YES] I have updated the documentation accordingly.
* [YES] I have added te...## All Submissions:
-------------------------------------
* [YES/NO] I have added an explanation of what changes in this merge do and why we should include it?
* [YES] I have updated the documentation accordingly.
* [YES] I have added tests to cover my changes.
* [YES] All new and existing tests passed.
* [YES/NO/NA] My code follows the code style of this project.
* [YES/NO/NA] I ran lint checks locally prior to submission.
## What is the issue or story related to the change?
-------------------------------------
<!-- Please describe the current behavior that you are modifying, 'or' link to a relevant issue.
Feel free to add references to any design documents you might have shared with the team or any
related MR that you are building on top of. -->
Currently, there are no logs indicating what components causes health checks to fail.
With the changes made, we can see the logs.
https://dev.azure.com/msazure/One/_sprints/taskboard/Azure%20Global%20Engineering%20Energy%20Team/One/AGE/Cobalt/Jun?workitem=10046151
High level design:
Issue: <!-- Link any __GitLab__ workitem(s) to this pull request. -->
<!-- Please add implementation details of current set of changes and how the code changes are
doing what they are expected to do. Are there any complex loops or designated code blocks that
should be elaborated? Is there some contextual knowledge that the reviewer should be aware of? -->
Change details:
## Test coverage:
------------------
<!-- Mention unit test coverage of changes. -->
## Does this introduce a breaking change?
-------------------------------------
- [NO]
<!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. -->
## Pending items
----------------
<!-- Are there changes that you'll introduce in upcoming MRs and hence did not add in this one? Next steps of your
feature can also be mentioned here. -->
## Reviewer request
-------------------
- Please provide an ETA when you plan to review this MR. Write a comment to decline or provide an ETA.
- Block the MR if you feel there is less testing or no details in the MR
- Please cover the following aspects in the MR
-- Coding design: _\<Reviewer1>_
-- Backward Compatibility: _\<Reviewer2>_
-- Feature Logic: _\<Logic design\>_
-- _\<Any other context mention here>_
OR
-- _\<Component 1>_: _\<Reviewer1>_
-- _\<CosmosDB>_: _\<Reviewer2>_
-- _\<ServiceBus>_ _\<Reviewer3>_
-- _\<Mention any other component and owner>_
## Other information
-------------------------------------
<!-- Any other information that is important to this MR such as screenshots of how the component looks before and after the change. -->
This is the log seen when tested locally with storage service.
2021-06-10 16:40:26.858 ERROR MININT-DBDIKLJ --- [nio-8081-exec-2] o.o.o.a.h.ActuatorHealthLogger correlation-id= data-partition-id=: Health component keyVault has status DOWN
![image](/uploads/0a6c2099e9a981d98fd452cfbd56ec74/image.png)M7 - Release 0.10Smitha ManjunathSmitha Manjunathhttps://community.opengroup.org/osdu/platform/system/lib/cloud/azure/os-core-lib-azure/-/merge_requests/118Overloading existing methods in CosmosStore and BlobStore class for system re...2023-08-18T12:44:31ZAman VermaOverloading existing methods in CosmosStore and BlobStore class for system resourcesProblem:
====
The `cosmosStore` class today is the facade to perform CRUD on data partition related cosmos dbs. All the methods in this class are tightly coupled with data-partition-id. Since the system data resources won't be coupled wi...Problem:
====
The `cosmosStore` class today is the facade to perform CRUD on data partition related cosmos dbs. All the methods in this class are tightly coupled with data-partition-id. Since the system data resources won't be coupled with a data-partition-id, the existing facade can't support CRUD on system resources. Same problem exists with `BlobStore` class
Changes in MR:
===
1. Enhanced the current CosmosClientFactory to return system level cosmos clients too.
2. Enhanced the current BlobServiceClientFactory to return system level cosmos clients too.
3. Overloaded existing methods. The new methods don't accept data-partition-id parameter
4. Minor code refactoring
5. Added UTsM7 - Release 0.10Aman VermaAman Vermahttps://community.opengroup.org/osdu/platform/system/lib/cloud/azure/os-core-lib-azure/-/merge_requests/121Added configuration related to airflow in PartitionInfoAzure2023-08-18T12:44:30ZVineeth Guna [Microsoft]Added configuration related to airflow in PartitionInfoAzureM7 - Release 0.10https://community.opengroup.org/osdu/platform/system/lib/cloud/azure/os-core-lib-azure/-/merge_requests/125Add circuitbreaker to all inter service communication2023-08-18T12:44:28ZRonak SakhujaAdd circuitbreaker to all inter service communication## All Submissions:
-------------------------------------
* [YES] I have added an explanation of what changes in this merge do and why we should include it?
* [NO] I have updated the documentation accordingly.
* [YES] I have added tests ...## All Submissions:
-------------------------------------
* [YES] I have added an explanation of what changes in this merge do and why we should include it?
* [NO] I have updated the documentation accordingly.
* [YES] I have added tests to cover my changes.
* [YES] All new and existing tests passed.
* [YES] My code follows the code style of this project.
* [YES] I ran lint checks locally prior to submission.
## What is the issue or story related to the change?
-------------------------------------
We are adding a circuitbreaker to all Interservice communication. All Services use HTTPClient's send method to make requests. In this MR, we are decorating the send method with a circuibreaker to avoid the problem of cascading fallback.
Be default, we are using default configurations of circuibreaker. Values can be found here(https://resilience4j.readme.io/docs/circuitbreaker#create-and-configure-a-circuitbreaker)
High level design:
![image](/uploads/6e831b2eafffb873f6f38632b4a69339/image.png)
CircuitBreaker is at the service level. At pod level communication, the URL for let's say entitlements service is defined as : http://entitlements/api/entitlements/v2 . We will fetch the host and make it as the key for circuitbreaker.
Issue: <!-- Link any __GitLab__ workitem(s) to this pull request. -->
<!-- Please add implementation details of current set of changes and how the code changes are
doing what they are expected to do. Are there any complex loops or designated code blocks that
should be elaborated? Is there some contextual knowledge that the reviewer should be aware of? -->
Change details:
We are adding a circuitbreakersingleton class to keep the circuitbreakerregistry. There is a flag which can be set to enable circuitbreaker for a service.
When circuitbreaks, it will start throwing error 500 without making http call.
## Test coverage & Tests:
------------------
<!-- Mention unit test coverage of changes. -->
Used Storage service and Entitlements service for testing.
While Entitlements was up, sent HttpRequests from Storage to Entitlements with success response.
Turned off Entitlements service and then sent requests from storage to entitlements which resulted in errors.
After the required number of calls, obtained log that circuit has opened and post which the requests were failing fast.
Tested the behavior by running on kubernetes cluster locally.
## Test on perf environment
I setup Storage and Entitlements on perf, and added a preauthorize on healthcheck in storage.
Scenario : Storage service calling entitlements
1. Behaviour when Entitlements is up :
![image](/uploads/194977dcf756eac71598260ec8226c7b/image.png)
2. Behaviour when Entitlements is down and circuit is closed :
![image](/uploads/aa6813f973711dd823d2aa86576433de/image.png)
3. Behaviour when Failure threshold is crossed and circuit is open :
![image](/uploads/6afaddc06ddf30d92d48f4d474cc09a1/image.png)
UPDATE : When Circuit is open, we are throwing Exception 503 : Service Unavailable
Logs generated when circuit gets open :
![image](/uploads/93788b1e68e14996451f02c5b8490897/image.png)
Tested to ensure multiple circuitbreakers for different services can be maintained.
## Does this introduce a breaking change?
-------------------------------------
- NO
<!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. -->
## Pending items
----------------
1. Make all configuration of circuitbreaker configurable.
2. Segregate circuitbreakers to remove parameters from circuitbreaker name
## Reviewer request
-------------------
- Please provide an ETA when you plan to review this MR. Write a comment to decline or provide an ETA.
- Block the MR if you feel there is less testing or no details in the MR
- Please cover the following aspects in the MR
-- Coding design: _\<Reviewer1>_
-- Backward Compatibility: _\<Reviewer2>_
-- Feature Logic: _\<Logic design\>_
-- _\<Any other context mention here>_
OR
-- _\<Component 1>_: _\<Reviewer1>_
-- _\<CosmosDB>_: _\<Reviewer2>_
-- _\<ServiceBus>_ _\<Reviewer3>_
-- _\<Mention any other component and owner>_
## Other information
-------------------------------------
<!-- Any other information that is important to this MR such as screenshots of how the component looks before and after the change. -->M8 - Release 0.11https://community.opengroup.org/osdu/platform/system/lib/cloud/azure/os-core-lib-azure/-/merge_requests/126added docs folder2023-08-18T12:44:26ZSmitha Manjunathadded docs folder## All Submissions:
-------------------------------------
* [YES] I have added an explanation of what changes in this merge do and why we should include it?
* [YES/NO] I have updated the documentation accordingly.
* [NA] I have added tes...## All Submissions:
-------------------------------------
* [YES] I have added an explanation of what changes in this merge do and why we should include it?
* [YES/NO] I have updated the documentation accordingly.
* [NA] I have added tests to cover my changes.
* [NA] All new and existing tests passed.
* [NA] My code follows the code style of this project.
* [NA] I ran lint checks locally prior to submission.
## What is the issue or story related to the change?
-------------------------------------
<!-- Please describe the current behavior that you are modifying, 'or' link to a relevant issue.
Feel free to add references to any design documents you might have shared with the team or any
related MR that you are building on top of. -->
High level design:
Issue: <!-- Link any __GitLab__ workitem(s) to this pull request. -->
<!-- Please add implementation details of current set of changes and how the code changes are
doing what they are expected to do. Are there any complex loops or designated code blocks that
should be elaborated? Is there some contextual knowledge that the reviewer should be aware of? -->
Change details:
## Test coverage:
------------------
<!-- Mention unit test coverage of changes. -->
## Does this introduce a breaking change?
-------------------------------------
- [NO]
<!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. -->
## Pending items
----------------
<!-- Are there changes that you'll introduce in upcoming MRs and hence did not add in this one? Next steps of your
feature can also be mentioned here. -->
## Reviewer request
-------------------
- Please provide an ETA when you plan to review this MR. Write a comment to decline or provide an ETA.
- Block the MR if you feel there is less testing or no details in the MR
- Please cover the following aspects in the MR
-- Coding design: _\<Reviewer1>_
-- Backward Compatibility: _\<Reviewer2>_
-- Feature Logic: _\<Logic design\>_
-- _\<Any other context mention here>_
OR
-- _\<Component 1>_: _\<Reviewer1>_
-- _\<CosmosDB>_: _\<Reviewer2>_
-- _\<ServiceBus>_ _\<Reviewer3>_
-- _\<Mention any other component and owner>_
## Other information
-------------------------------------
<!-- Any other information that is important to this MR such as screenshots of how the component looks before and after the change. -->M7 - Release 0.10Smitha ManjunathSmitha Manjunathhttps://community.opengroup.org/osdu/platform/system/lib/cloud/azure/os-core-lib-azure/-/merge_requests/128patch to conditionally enable this bean.2023-08-18T12:44:24ZSmitha Manjunathpatch to conditionally enable this bean.## All Submissions:
-------------------------------------
* [YES/NO] I have added an explanation of what changes in this merge do and why we should include it?
* [YES/NO] I have updated the documentation accordingly.
* [YES/NO/NA] I have...## All Submissions:
-------------------------------------
* [YES/NO] I have added an explanation of what changes in this merge do and why we should include it?
* [YES/NO] I have updated the documentation accordingly.
* [YES/NO/NA] I have added tests to cover my changes.
* [YES/NO/NA] All new and existing tests passed.
* [YES/NO/NA] My code follows the code style of this project.
* [YES/NO/NA] I ran lint checks locally prior to submission.
## What is the issue or story related to the change?
-------------------------------------
<!-- Please describe the current behavior that you are modifying, 'or' link to a relevant issue.
Feel free to add references to any design documents you might have shared with the team or any
related MR that you are building on top of. -->
High level design:
Issue: <!-- Link any __GitLab__ workitem(s) to this pull request. -->
<!-- Please add implementation details of current set of changes and how the code changes are
doing what they are expected to do. Are there any complex loops or designated code blocks that
should be elaborated? Is there some contextual knowledge that the reviewer should be aware of? -->
Change details:
## Test coverage:
------------------
<!-- Mention unit test coverage of changes. -->
## Does this introduce a breaking change?
-------------------------------------
- [YES/NO]
<!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. -->
## Pending items
----------------
<!-- Are there changes that you'll introduce in upcoming MRs and hence did not add in this one? Next steps of your
feature can also be mentioned here. -->
## Reviewer request
-------------------
- Please provide an ETA when you plan to review this MR. Write a comment to decline or provide an ETA.
- Block the MR if you feel there is less testing or no details in the MR
- Please cover the following aspects in the MR
-- Coding design: _\<Reviewer1>_
-- Backward Compatibility: _\<Reviewer2>_
-- Feature Logic: _\<Logic design\>_
-- _\<Any other context mention here>_
OR
-- _\<Component 1>_: _\<Reviewer1>_
-- _\<CosmosDB>_: _\<Reviewer2>_
-- _\<ServiceBus>_ _\<Reviewer3>_
-- _\<Mention any other component and owner>_
## Other information
-------------------------------------
<!-- Any other information that is important to this MR such as screenshots of how the component looks before and after the change. -->M8 - Release 0.11Smitha ManjunathSmitha Manjunathhttps://community.opengroup.org/osdu/platform/system/lib/cloud/azure/os-core-lib-azure/-/merge_requests/131KV throttling issue - improve caching2023-08-18T12:44:22ZAlok JoshiKV throttling issue - improve caching## All Submissions:
-------------------------------------
* [YES] I have added an explanation of what changes in this merge do and why we should include it?
* [NA] I have updated the documentation accordingly.
* [YES] I have added tests ...## All Submissions:
-------------------------------------
* [YES] I have added an explanation of what changes in this merge do and why we should include it?
* [NA] I have updated the documentation accordingly.
* [YES] I have added tests to cover my changes.
* [YES] All new and existing tests passed.
* [YES] My code follows the code style of this project.
* [YES] I ran lint checks locally prior to submission.
## What is the issue or story related to the change?
-------------------------------------
<!-- Please describe the current behavior that you are modifying, 'or' link to a relevant issue.
Feel free to add references to any design documents you might have shared with the team or any
related MR that you are building on top of. -->
Issue: If the system is under moderate/heavy load, we are seeing a lot of issues with Storage service because of KV throttling. The error we see is:
`storage.app Status code 429, "{"error":{"code":"Throttled","message":"Request was not processed because too many requests were received. Reason: VaultOperationLimitReached"}}"`
<!-- Please add implementation details of current set of changes and how the code changes are
doing what they are expected to do. Are there any complex loops or designated code blocks that
should be elaborated? Is there some contextual knowledge that the reviewer should be aware of? -->
Change details:
To address this, we can follow the [kv-throttling](https://docs.microsoft.com/en-us/azure/key-vault/general/overview-throttling) guideline from MSFT which involves:
- Retry policy when creating the secret client (EDIT: Removed after comments, to be considered in a separate MR)
- Improve caching when Storage calls `publishToEventGridTopic`
## Test coverage:
------------------
<!-- Mention unit test coverage of changes. -->
## Does this introduce a breaking change?
-------------------------------------
- [NO]
<!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. -->
## Pending items
----------------
<!-- Are there changes that you'll introduce in upcoming MRs and hence did not add in this one? Next steps of your
feature can also be mentioned here. -->
## Reviewer request
-------------------
- Please provide an ETA when you plan to review this MR. Write a comment to decline or provide an ETA.
- Block the MR if you feel there is less testing or no details in the MR
- Please cover the following aspects in the MR
-- Coding design: _\<Reviewer1>_
-- Backward Compatibility: _\<Reviewer2>_
-- Feature Logic: _\<Logic design\>_
-- _\<Any other context mention here>_
OR
-- _\<Component 1>_: _\<Reviewer1>_
-- _\<CosmosDB>_: _\<Reviewer2>_
-- _\<ServiceBus>_ _\<Reviewer3>_
-- _\<Mention any other component and owner>_
## Other information
-------------------------------------
<!-- Any other information that is important to this MR such as screenshots of how the component looks before and after the change. -->M8 - Release 0.11Alok JoshiAlok Joshihttps://community.opengroup.org/osdu/platform/system/lib/cloud/azure/os-core-lib-azure/-/merge_requests/132Updating Factory Implementations for Client generation using Key vault Creden...2023-08-18T12:44:19Zharshit aggarwalUpdating Factory Implementations for Client generation using Key vault Credentials## All Submissions:
-------------------------------------
* [YES] I have added an explanation of what changes in this merge do and why we should include it?
* [YES] I have updated the documentation accordingly.
* [YES] I have added tests...## All Submissions:
-------------------------------------
* [YES] I have added an explanation of what changes in this merge do and why we should include it?
* [YES] I have updated the documentation accordingly.
* [YES] I have added tests to cover my changes.
* [YES] All new and existing tests passed.
* [YES] My code follows the code style of this project.
* [YES] I ran lint checks locally prior to submission.
## What is the issue or story related to the change?
-------------------------------------
Currently we are using MSI to connect with Azure Resources in Data Partition, which requires appropriate role assignments of Service Principal and Managed Identities over the respective Azure resources
As of now the [DefaultAzureCredential ](https://community.opengroup.org/osdu/platform/system/lib/cloud/azure/os-core-lib-azure/-/blob/master/src/main/java/org/opengroup/osdu/azure/dependencies/AzureOSDUConfig.java#L58) class is getting used to generate AD Tokens for Authenticating to Azure Resources
This change is updating Factory Implementations for Client Generation using KeyVault Credentials
Storage Account Blob Clients & Service Bus Topic Clients are getting initialized using DefaultAzureCredential and corresponding implementation using Keyvault Credentials is introduced in the change
The change is under feature flag and can be controlled via the property `azure.msi.isEnabled`
- By default the existing set up of MSI will continue to work as `matchIfMissing` property is set to `true`
- This property has to be set to `false` to use the clients using KeyVault Credentials
- Services willing to use clients with KeyVault Credentials should set `azure.msi.isEnabled=false` in application.properties
**The change has been tested by including this version of core-lib-azure in Schema, Storage & WKS service locally**
## Test coverage:
------------------
## Does this introduce a breaking change?
-------------------------------------
- [NO]
## Pending items
----------------
## Reviewer request
-------------------
- Please provide an ETA when you plan to review this MR. Write a comment to decline or provide an ETA.
- Block the MR if you feel there is less testing or no details in the MR
- Please cover the following aspects in the MR
-- Coding design: _\<Reviewer1>_
-- Backward Compatibility: _\<Reviewer2>_
-- Feature Logic: _\<Logic design\>_
-- _\<Any other context mention here>_
OR
-- _\<Component 1>_: _\<Reviewer1>_
-- _\<CosmosDB>_: _\<Reviewer2>_
-- _\<ServiceBus>_ _\<Reviewer3>_
-- _\<Mention any other component and owner>_
## Other information
-------------------------------------M8 - Release 0.11harshit aggarwalharshit aggarwalhttps://community.opengroup.org/osdu/platform/system/lib/cloud/azure/os-core-lib-azure/-/merge_requests/133Fix NOTICE file issue2023-08-18T12:44:18ZAlok JoshiFix NOTICE file issueNo code change, updating NOTICE file to fix build issueNo code change, updating NOTICE file to fix build issueM8 - Release 0.11Alok JoshiAlok Joshi