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

add -db flag for persistent data #155

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

banksean
Copy link

This hack adds a --db <path> flag so that the duckdb instance is persisted to disk between otel-desktop-viewer runs instead of disappearing forever when the process exits.

make run-go-db

Will create /tmp/otel.db if it doesn't exist, and load its contents if it does. Traces that show up in the viewer should be persisted to that file.

After stopping the viewer (maybe before that even, thought it's probably not safe), you should be able to query the trace data in that file using the duckdb cli:

> duckdb /tmp/otel.db                                                                                                            
v0.10.1 4a89d97db8
Enter ".help" for usage hints.
D .schema
CREATE TABLE spans(traceID VARCHAR, traceState VARCHAR, spanID VARCHAR, parentSpanID VARCHAR, "name" VARCHAR, kind VARCHAR, startTime TIMESTAMP_NS, endTime TIMESTAMP_NS, attributes JSON, events JSON, links JSON, resourceAttributes JSON, resourceDroppedAttributesCount UINTEGER, scopeName VARCHAR, scopeVersion VARCHAR, scopeAttributes JSON, scopeDroppedAttributesCount UINTEGER, droppedAttributesCount UINTEGER, droppedEventsCount UINTEGER, droppedLinksCount UINTEGER, statusCode VARCHAR, statusMessage VARCHAR);
D select count(*) from spans;
┌──────────────┐
│ count_star() │
│    int64     │
├──────────────┤
│           50 │
└──────────────┘
D 

The implementation for this change is admittedly a mess -I don't quite understand the generated code bits work in this repo, so I imagine these changes would probably get clobbered by the next go generate run anyways.

Note: this could probably have been more easily implemented via environment variables, and have store.go read those directly instead of trying to pass cli flags through all the layers of config and factory indirection.

@@ -17,20 +17,23 @@ const (
)

// Creates a factory for the Desktop Exporter
func NewFactory() exporter.Factory {
func NewFactory(dbFilename string) exporter.Factory {
Copy link
Contributor

@nopcoder nopcoder Nov 16, 2024

Choose a reason for hiding this comment

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

Not qualified to do a full review here. but I think there is no need to add dbFilename as argument here and pass it to createDefaultConfig.
If you add a line in main.go as part of set.ConfigProviderSettings.ResolverSettings.URIs you can set a the value for the db filename using yaml:exporters::desktop::dbfilename: + dbFilenameFlag

Can scope the flag variable to the newCommand and have mapstructure populate the value based on the flag.

This way you can skip the value passing and have the value populated based on the flag.

Hope it helps to simplify the code

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.

2 participants