Skip to content

Initial implementation#1

Merged
kaklakariada merged 25 commits intomainfrom
initial-implementation
Apr 15, 2026
Merged

Initial implementation#1
kaklakariada merged 25 commits intomainfrom
initial-implementation

Conversation

@kaklakariada
Copy link
Copy Markdown
Collaborator

@kaklakariada kaklakariada commented Apr 13, 2026

Closes #2

Copy link
Copy Markdown

@antonireus antonireus left a comment

Choose a reason for hiding this comment

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

In general the code is quite good. The code for "closing" feels a bit odd at the begining, but looking at the specs this arises for the requirment of the shutdown process has to send the remaining messages.

The only issue would be that for a telemetry library I think we should aim at having a lower overhead. Some would label this as "premature optimization", but I think there are some good practives that should be followed to avoid some unnecessary allocations and computations, that wouldn't make the code less readable.

Perhaps, what is missing is a performance requirement in the speq, something like the library should have a low overhead, and avoid memory allocations if possible.... and when tracing is disabled the tracing should be a no-op wihtout overhead.

Also clarifying what can be passed as feature would help. This will likely be constant strings, whitout special characters, so it doesn't make sense that the library spends compute with trimming calls, and making copyies to escape chars that won't be there.

Finally, on the test side, I'm missing tests to verify that the JSON payloads are wellformed.

Comment thread src/main/java/com/exasol/telemetry/TelemetryClient.java
Comment thread src/main/java/com/exasol/telemetry/Message.java
Comment thread src/test/java/com/exasol/telemetry/TrackingApiIT.java
Comment thread src/test/java/com/exasol/telemetry/MessageTest.java
Comment thread src/main/java/com/exasol/telemetry/TelemetryClient.java
Comment thread src/main/java/com/exasol/telemetry/TelemetryClient.java Outdated
@kaklakariada
Copy link
Copy Markdown
Collaborator Author

@antonireus I created a separate issue for validating feature names, as it's not yet clear how to implement this: #4

@kaklakariada kaklakariada merged commit aea9dad into main Apr 15, 2026
4 checks passed
@kaklakariada kaklakariada deleted the initial-implementation branch April 15, 2026 09:30
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.

Create MVP

2 participants