Rework otel crud storage#826
Conversation
…rage # Conflicts: # ladybug-backend-jaxrs/pom.xml # ladybug-backend-jaxrs/src/main/java/org/wearefrank/ladybug/web/jaxrs/api/TracingApi.java # ladybug-backend-springmvc/pom.xml # ladybug-backend-springmvc/src/main/java/org/wearefrank/ladybug/web/springmvc/api/TracingApi.java # ladybug-common/pom.xml # ladybug-common/src/main/java/org/wearefrank/ladybug/web/common/TracingApiImpl.java
|
|
||
| ``` | ||
| <property name="openTelemetryEndpoint" ref="openTelemetryEndpoint"/> | ||
| ``` |
There was a problem hiding this comment.
I would like to have an end-to-end test some time with Cypress. I also would like to have end-to-end tests with openTelemetryEndpoint enabled and test environments with openTelemetryEndpoint disabled. Would be nice if that could be added to this PR, but in my opinion it can also be done in a later PR.
| import java.util.Objects; | ||
| import java.util.Scanner; | ||
| import java.util.Set; | ||
| import java.util.*; |
There was a problem hiding this comment.
I prefer explicit import, for example java.util.List or java.util.ArrayList or something. I would like to see explicitly what you are importing from each package. I think this may help me in the future when I see a classname somewhere and I want to know what package it is coming from.
| } | ||
|
|
||
| @Test | ||
| public void testOrphanGetsReparentedLater() { |
There was a problem hiding this comment.
Nice test. This is quite helpful.
| <!-- Use memory storage by default as Cypress tests depend on memory storage / starting with empty storages --> | ||
|
|
||
| <bean name="debugStorage" class="org.wearefrank.ladybug.storage.memory.MemoryLogStorage"> | ||
| <bean name="debugStorage" class="org.wearefrank.ladybug.storage.memory.MemoryCrudStorage"> |
There was a problem hiding this comment.
The debug storage should be a LogStorage because otherwise users can change reports in the debug tab which is not desired. This emphasizes the need to have a different environment for testing OpenTelemetry.
|
|
||
| @Override | ||
| public void store(Report report) throws StorageException { | ||
| storeWithoutException(report); |
There was a problem hiding this comment.
I wouldn't expect this to be necessary. A log storage has storeWithException() to store reports.
| @@ -18,18 +18,7 @@ | |||
| import java.beans.Transient; | |||
There was a problem hiding this comment.
There are new transient fields of Report and Checkpoint. These are not persisted when you download and upload reports. Are all use cases covered about download and upload for OpenTelementry and non-OpenTelemetry reports?
| @@ -2,7 +2,6 @@ | |||
| <%@ page import="org.wearefrank.ladybug.TestTool"%> | |||
There was a problem hiding this comment.
Can you add links here to create OpenTelemetry stuff (spans? traces?)?
…rage # Conflicts: # ladybug-backend-jaxrs/src/main/java/org/wearefrank/ladybug/web/jaxrs/api/TracingApi.java # ladybug-backend-springmvc/src/main/java/org/wearefrank/ladybug/web/springmvc/api/TracingApi.java # ladybug-common/pom.xml # ladybug-common/src/main/java/org/wearefrank/ladybug/TestTool.java # ladybug-common/src/main/java/org/wearefrank/ladybug/web/common/TracingApiImpl.java
2 participants
Processes spans to reports. Steps: