OS Core Common - Spring 5 issueshttps://community.opengroup.org/osdu/platform/system/lib/core/os-core-common/-/issues2023-09-18T07:26:13Zhttps://community.opengroup.org/osdu/platform/system/lib/core/os-core-common/-/issues/72Error getting data from Redis on lost connection2023-09-18T07:26:13ZYurii Ruban [EPAM / GCP]Error getting data from Redis on lost connectionIn case of loss of connection between the service and Redis, the processing of requests to the service is delayed and an error of type
`Command timed out after 30 SECONDS` is received.
Redis (cache service) is a productivity tool. In c...In case of loss of connection between the service and Redis, the processing of requests to the service is delayed and an error of type
`Command timed out after 30 SECONDS` is received.
Redis (cache service) is a productivity tool. In case of a lost connection, delays in processing requests increase, and, in general, the service becomes unviable.
`entitlements-v2.app: An unknown error has occurred
AppException(error=AppError(code=500, reason=Internal Server Error, message=An unknown error has occurred, errors=null, debuggingInfo=null, originalException=com.lambdaworks.redis.RedisCommandTimeoutException: Command timed out after 30 SECONDS), originalException=com.lambdaworks.redis.RedisCommandTimeoutException: Command timed out after 30 SECONDS)
...
Caused by: com.lambdaworks.redis.RedisCommandTimeoutException: Command timed out after 30 SECONDS
`
I recommend adding Redis health checks.M20 - Release 0.23Yurii Ruban [EPAM / GCP]Yurii Ruban [EPAM / GCP]https://community.opengroup.org/osdu/platform/system/lib/core/os-core-common/-/issues/70Invalid data-partition-id will create 500 code with no Authorize message.2023-10-10T12:16:06ZBruce JinInvalid data-partition-id will create 500 code with no Authorize message.While making calls to OSDU services, such as `secret` and `storage` service, testers discover that if they put invalid symbols in `data_partition_id`, we will have 500 code, but with reason of Access Denied.
After investigation, we reali...While making calls to OSDU services, such as `secret` and `storage` service, testers discover that if they put invalid symbols in `data_partition_id`, we will have 500 code, but with reason of Access Denied.
After investigation, we realize the partition service did not consider the situation that user put invalid URI symbols like `@#$%` in data partition
id, which will make the `normalizeStringUrl` function have Java.Lang exception in this [UrlNormalizationUtil.java](https://community.opengroup.org/osdu/platform/system/lib/core/os-core-common/-/blob/master/src/main/java/org/opengroup/osdu/core/common/util/UrlNormalizationUtil.java).
```
Caused by: java.lang.IllegalArgumentException: Malformed escape pair at index 57: http://os-partition:8080/api/partition/v1/partitions/osdu%
at java.net.URI.create(URI.java:852)
at org.opengroup.osdu.core.common.util.UrlNormalizationUtil.normalizeStringUrl(UrlNormalizationUtil.java:27)
```
This will generate a 500 code in entitlement service since the service will treat this error as a general error in [SpringExceptionMapper.java](handleGeneralException), instead a 400 code.
Also in Entitlements the error message is processed within [AuthorizationServiceImpl.java](https://community.opengroup.org/osdu/platform/system/lib/core/os-core-common/-/blob/master/src/main/java/org/opengroup/osdu/core/common/entitlements/AuthorizationServiceImpl.java), so it will have `"Access denied", "The user is not authorized to perform this action"` error message.
Here is a MR that will handle the 500 code produced from `java.lang.IllegalArgumentException` https://community.opengroup.org/osdu/platform/system/lib/core/os-core-common/-/merge_requests/219Bruce JinBruce Jinhttps://community.opengroup.org/osdu/platform/system/lib/core/os-core-common/-/issues/68Core-common post migration TODOs (Powermock, module access)2023-06-28T14:57:27ZRustam Lotsmanenko (EPAM)rustam_lotsmanenko@epam.comCore-common post migration TODOs (Powermock, module access)- Unit test refactoring is required, to remove --add-opens params from build run args.
~~~
<argLine>--add-opens java.base/java.lang=ALL-UNNAMED --add-opens java.base/java.util=ALL-UNNAMED</argLine>
~~~
Tests failed without args:
~~~
Fail...- Unit test refactoring is required, to remove --add-opens params from build run args.
~~~
<argLine>--add-opens java.base/java.lang=ALL-UNNAMED --add-opens java.base/java.util=ALL-UNNAMED</argLine>
~~~
Tests failed without args:
~~~
Failed tests:
shouldThrowExceptionWhenPropertyNotPresentInEnv(org.opengroup.osdu.core.common.partition.PartitionPropertyResolverTest)
shouldThrowExceptionWhenPropertyInMapNull(org.opengroup.osdu.core.common.partition.PartitionPropertyResolverTest)
shouldThrowExceptionWhenPropertyValueInMapNull(org.opengroup.osdu.core.common.partition.PartitionPropertyResolverTest)
shouldThrowExceptionWhenPropertyNotInMap(org.opengroup.osdu.core.common.partition.PartitionPropertyResolverTest)
Tests in error:
shouldNotThrowExceptionWhenPropertyValueInMapIsEmpty(org.opengroup.osdu.core.common.partition.PartitionPropertyResolverTest)
shouldNotThrowExceptionWhenEnvReturnEmptyVal(org.opengroup.osdu.core.common.partition.PartitionPropertyResolverTest)
shouldGetTypedProperties(org.opengroup.osdu.core.common.partition.PartitionPropertyResolverTest)
getPropertyValueFromEnv(org.opengroup.osdu.core.common.partition.PartitionPropertyResolverTest)
shouldThrowExceptionWhenEnvReturnNull(org.opengroup.osdu.core.common.partition.PartitionPropertyResolverTest)
getPropertyValueFromMapValue(org.opengroup.osdu.core.common.partition.PartitionPropertyResolverTest)
~~~
- Powermock dependency removal is highly recommended, Powermock not getting updates to work with Java 17, and has conflicts with Mockito:
~~~
<dependency>
<groupId>org.powermock</groupId>
<artifactId>powermock-core</artifactId>
<version>2.0.9</version>
<scope>test</scope>
</dependency>
~~~https://community.opengroup.org/osdu/platform/system/lib/core/os-core-common/-/issues/67inefficient/non-performant crs-conversions causing reliability/performance is...2023-06-21T07:54:16ZYurii Kondakovinefficient/non-performant crs-conversions causing reliability/performance issues on ingestion workflowWe are seeing reliability and performance issue because of inefficient/non-performant crs-conversions on ingestion workflow. If crs conversions takes long time (on Storage /batch API), then it slows down entire ingestion workflow.
There...We are seeing reliability and performance issue because of inefficient/non-performant crs-conversions on ingestion workflow. If crs conversions takes long time (on Storage /batch API), then it slows down entire ingestion workflow.
There is a need for setting a timeout for crs-conversion requests that run for more than a certain time. For the requests to crs-conversion-service currently java.net.HttpURLConnection class is used, which is only has connectionTimeout and readTimeout properties, that don't help us to set the timeout.
It is suggested to use apache CloseableHttpClient httpClient that has socketTimeout property.
core-common MR - https://community.opengroup.org/osdu/platform/system/lib/core/os-core-common/-/merge_requests/213
storage MR - https://community.opengroup.org/osdu/platform/system/storage/-/merge_requests/712M19 - Release 0.22Yurii KondakovYurii Kondakovhttps://community.opengroup.org/osdu/platform/system/lib/core/os-core-common/-/issues/64Spring configuration related to Feature flag should be optional2023-01-30T11:37:15ZRustam Lotsmanenko (EPAM)rustam_lotsmanenko@epam.comSpring configuration related to Feature flag should be optionalIt is possible that some OSDU services may not use Partition service, Unit service as an example. But the configuration of the recently merged code https://community.opengroup.org/osdu/platform/system/lib/core/os-core-common/-/merge_requ...It is possible that some OSDU services may not use Partition service, Unit service as an example. But the configuration of the recently merged code https://community.opengroup.org/osdu/platform/system/lib/core/os-core-common/-/merge_requests/189 requires PARTITION_API env variable mandatorily. It will be better to change the configuration to make it optional, instead of bringing this variable to each service, no matter if Partition is used or not.
~~~
Exception encountered during context initialization - cancelling refresh attempt:
org.springframework.beans.factory.BeanCreationException:
Error creating bean with name 'featureFlagConfig':
Injection of autowired dependencies failed; nested exception is java.lang.IllegalArgumentException:
Could not resolve placeholder 'PARTITION_API' in value "${PARTITION_API}"
~~~Rustam Lotsmanenko (EPAM)rustam_lotsmanenko@epam.comRiabokon Stanislav(EPAM)[GCP]Rustam Lotsmanenko (EPAM)rustam_lotsmanenko@epam.comhttps://community.opengroup.org/osdu/platform/system/lib/core/os-core-common/-/issues/10Expose audit trail attributes in search results2021-10-04T22:12:09ZDebasis ChatterjeeExpose audit trail attributes in search resultsThis is very useful information and may be added to standard query result.
Username who created the record.
Date/time when record was created in OSDU.
Also expose Username who updated the record.
Date/time when update operation was per...This is very useful information and may be added to standard query result.
Username who created the record.
Date/time when record was created in OSDU.
Also expose Username who updated the record.
Date/time when update operation was performed.
@dmitry-kniazev later added the example from IBM implementation as he was showing during recent session of "Reporting and dashboarding"?JoeJoehttps://community.opengroup.org/osdu/platform/system/lib/core/os-core-common/-/issues/5search.Config class in os-core-common does not respect Spring configuration2021-01-17T16:29:09ZDmitriy Rudkosearch.Config class in os-core-common does not respect Spring configuration## Change Type:
- [ ] Feature
- [x] Bugfix
- [x] Refactoring
## Context and Scope
Implementation of `org.opengroup.osdu.core.common.search.Config` class make it hard to use class in modules and introduce several implicit issues:
1. Spr...## Change Type:
- [ ] Feature
- [x] Bugfix
- [x] Refactoring
## Context and Scope
Implementation of `org.opengroup.osdu.core.common.search.Config` class make it hard to use class in modules and introduce several implicit issues:
1. Spring does not support ingestion of `@Values` in static fields. The fact that the class is annotated with Spring annotations (`@Component`, `@Value`) misled because class can be used only as a general Java class (`Config.QUERY_DEFAULT_LIMIT`)
2. Because Spring can't populate variables, configuration can be passed ONLY as Env variables.
3. The Class introduce implicit dependency on other Search related Variables in the code even module could not use them.
4. The Class introduce implicit dependency on GCP specific Variables for all CSPs. E.g. code of all CSPs will check presence of `GOOGLE_CLOUD_PROJECT` variable even it is GCP specific.
![Screen_Shot_2020-08-27_at_4.07.50_PM](/uploads/0e4f41a5ae23158ebebabe9803daaaff/Screen_Shot_2020-08-27_at_4.07.50_PM.png)
## Decision
- Move `org.opengroup.osdu.core.common.search.Config` class from os-core-common. Application configuration should be specified on microservice level, not in common library
## Rational
1. Dependency on Spring in common 'library' make it hard to use from hosting code
2. Dependency on `os-core-common` make application implicitly `aware` about all Spring components / variables declared in library
## Consequences
1. New `search.Config` class should be added in microservices that use the class (Search)
2. Changes in OS Core Common > QueryUtils.getResultSizeForQuery()ethiraj krishnamanaiduDania Kodeih (Microsoft)Wladmir FrazaoAlan Brazethiraj krishnamanaiduhttps://community.opengroup.org/osdu/platform/system/lib/core/os-core-common/-/issues/4Making UrlFetchServiceImpl as singleton spring bean2021-01-17T16:28:54ZKishore BattulaMaking UrlFetchServiceImpl as singleton spring bean# Convert http clients into spring singleton components
## Context and Scope
In current OSDU core commons there are two http clients
- UrlFetchServiceImpl which is implementation of IUrlFetchService. It uses HttpClientHandler which is a ...# Convert http clients into spring singleton components
## Context and Scope
In current OSDU core commons there are two http clients
- UrlFetchServiceImpl which is implementation of IUrlFetchService. It uses HttpClientHandler which is a spring with request scope
- HttpClient which is implementation of IHttpClient
UrlFetchServiceImpl is a spring component with request scope and HttpClient is not a spring component.
## Questions
- Any specific reason for having UrlFetchServiceImpl as request scoped spring bean? Is this because of injection of JaxRsDpsLogger in HttpClientHandler?
- Any specific reason for not having HttpClient as spring bean?
## Proposal
Convert these two http clients to spring bean with singleton as scope.
- Converting the existing UrlFetchServiceImpl and HttpClientHandler to singleton scope
- Converting the HttpClient as singleton scoped spring bean. We still support the constructor way of invoking for backward compatibility.
## Rationale
I am working on logging the request/response for external clients at these clients. For this logging I want to use JaxRsDpsLogger which needs to be autowired.
# ----------------- ADR update based on discussion ---------------
Splitting this ADR into two different issues.
This ADR is about converting UrlFetchServiceImpl as singleton spring bean.ethiraj krishnamanaiduDania Kodeih (Microsoft)Wladmir FrazaoBrandt BealAlan Brazethiraj krishnamanaiduhttps://community.opengroup.org/osdu/platform/system/lib/core/os-core-common/-/issues/1Adding support for logger name in JaxRsDpsLogger and ILogger for fine grained...2021-02-20T07:50:10ZKishore BattulaAdding support for logger name in JaxRsDpsLogger and ILogger for fine grained configuration of log messages## Context and Scope
Below are the sample logs in storage service
```
2020-06-29 19:34:19.783 INFO 5644 --- [nio-8082-exec-3] o.o.o.c.common.logging.DefaultLogWriter : storage.app: Start Web-API DELETE /records/opendes:id:1593439432354...## Context and Scope
Below are the sample logs in storage service
```
2020-06-29 19:34:19.783 INFO 5644 --- [nio-8082-exec-3] o.o.o.c.common.logging.DefaultLogWriter : storage.app: Start Web-API DELETE /records/opendes:id:1593439432354 Headers: {data-partition-id:opendes,content-type:application/json} {correlation-id=68a989a1-f1a2-4bbc-9f1c-5c734a790311, data-partition-id=opendes}
2020-06-29 19:34:22.342 INFO 5644 --- [nio-8082-exec-3] o.o.o.c.common.logging.DefaultLogWriter : {auditLog={resources=[opendes:id:1593439432354], action=DELETE, actionId=ST003, message=Record purged, user=e6ac6dfe-8198-4f9c-b2e8-b974c0f9ef5b, status=SUCCESS}} {correlation-id=68a989a1-f1a2-4bbc-9f1c-5c734a790311, data-partition-id=opendes}
2020-06-29 19:34:22.344 INFO 5644 --- [nio-8082-exec-3] o.o.o.c.common.logging.DefaultLogWriter : storage.app: Storage publishes message 68a989a1-f1a2-4bbc-9f1c-5c734a790311 {correlation-id=68a989a1-f1a2-4bbc-9f1c-5c734a790311, data-partition-id=opendes}
2020-06-29 19:34:22.670 INFO 5644 --- [nio-8082-exec-3] o.o.o.c.common.logging.DefaultLogWriter : storage.app: End Web-API DELETE /records/opendes:id:1593439432354 Headers: {correlation-id:68a989a1-f1a2-4bbc-9f1c-5c734a790311} timeTaken:2887 {correlation-id=68a989a1-f1a2-4bbc-9f1c-5c734a790311, data-partition-id=opendes}
```
If we notice above the loggerName for all the logs is `o.o.o.c.common.logging.DefaultLogWriter`. This is fully qualified name of the class in which logger instance is created by giving the class name as input.
Logging libraries like java.utils.logging, log4j2 etc., provides a way to configure log level based on logger name. As these logger names most of the times are full qualified names, this allows different configuration at package level or class level. Refer sample log4j2.xml below
```
<?xml version="1.0" encoding="UTF-8" ?>
<!DOCTYPE log4j:configuration SYSTEM "log4j.dtd">
<log4j:configuration xmlns:log4j="http://jakarta.apache.org/log4j/">
<appender name="console" class="org.apache.log4j.ConsoleAppender">
<param name="Target" value="System.out"/>
<layout class="org.apache.log4j.PatternLayout">
<param name="ConversionPattern" value="%-5p %c{1} - %m%n"/>
</layout>
</appender>
<logger name="com.example.mypackage" additivity="false">
<level value="info"/>
<appender-ref ref="console"/>
</logger>
<root>
<priority value="warn"/>
<appender-ref ref="console"/>
</root>
</log4j:configuration>
```
From the above configuration, for package `com.example.mypacage` we are logging any thing info and above where as for root we are logging warn and above.
Having loggerName will give fine grained control of log messages at different level like package or classes and different environments like dev, stage, prod.
## Decision
To support loggerName overloaded methods will be added to JaxRsDpsLogger.java and ILogger.java. ILogger.java new methods will have default implementation which will call into existing methods leaving the loggerName. This is to keep backward compatiability.
Cloud providers who what to use this additional feature will implement their own class for ILogger to support new loggerName.
## Rationale
This proposal will not affect the existing implementations and cloud providers can pick up these changes at their own pace if needed