Skip to content

Conversation

@spl3g
Copy link
Contributor

@spl3g spl3g commented Jun 17, 2025

closes #102
This PR adds support for storing filters and changing them in the dashboard entry form.
It adds 2 new tables for storing filters and a component for choosing them.

I wanted to make it so that the Downshift opens again if the tag is not fully finished, but I could not find a way to do this.
I am open to any suggestions regarding my code, and I haven't written any tests yet.. That's why it's a draft PR.

@spl3g spl3g force-pushed the filters branch 2 times, most recently from b2e1e2d to da667eb Compare June 17, 2025 21:27
@spl3g spl3g requested a review from jmattheis June 19, 2025 21:22
Copy link
Member

@jmattheis jmattheis left a comment

Choose a reason for hiding this comment

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

Currently, this branch doesn't compile:

$ yarn tsc
yarn run v1.22.22
$ /home/jm/src/traggo/server/ui/node_modules/.bin/tsc
src/tag/TagFilterSelector.tsx:99:13 - error TS2322: Type 'void' is not assignable to type '((selectedItem: any, stateAndHelpers: ControllerStateAndHelpers<any>) => void) | undefined'.

99             onChange={handleChange(item, state)}
               ~~~~~~~~

  node_modules/downshift/typings/index.d.ts:40:3
    40   onChange?: (
         ~~~~~~~~
    The expected type comes from property 'onChange' which is declared here on type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<DownshiftProps<any>, any, any>> & Readonly<...> & Readonly<...>'

src/tag/TagFilterSelector.tsx:99:36 - error TS2304: Cannot find name 'item'.

99             onChange={handleChange(item, state)}
                                      ~~~~

src/tag/TagFilterSelector.tsx:99:42 - error TS2304: Cannot find name 'state'.

99             onChange={handleChange(item, state)}
                                            ~~~~~


Found 3 errors.

error Command failed with exit code 1.

@spl3g
Copy link
Contributor Author

spl3g commented Jun 20, 2025

Oh, my bad. I don't know, how i missed that

@jmattheis
Copy link
Member

jmattheis commented Jun 21, 2025

FYI: There is a test failure and two unresolved conversations.

@spl3g
Copy link
Contributor Author

spl3g commented Aug 21, 2025

About the test still failing. The problem is with the sqlite hanging when trying to start a transaction. Removing the SetMaxOpenConns(1) fixes that, but i think that would break something else:

if dialect == "sqlite3" {
	// We use the database connection inside the handlers from the http
	// framework, therefore concurrent access occurs. Sqlite cannot handle
	// concurrent writes, so we limit sqlite to one connection.
	// see https://github.com/mattn/go-sqlite3/issues/274
	// db.DB().SetMaxOpenConns(1)
	db.Exec("PRAGMA foreign_keys = ON")
}

@spl3g spl3g mentioned this pull request Nov 24, 2025
@spl3g
Copy link
Contributor Author

spl3g commented Dec 15, 2025

Now that the tag selectors PR is merged, I can work on this one.

I noticed that when building Stats db query, OR is used both for include and exclude tags. I'm not sure that this is right for include tags.
in statistics/summary.go:

func (r *ResolverForStatistics) Stats(ctx context.Context, ranges []*gqlmodel.Range, tags []string, excludeTags []*gqlmodel.InputTimeSpanTag, requireTags []*gqlmodel.InputTimeSpanTag) ([]*gqlmodel.RangedStatisticsEntries, error) {
...
	queryRequire, requireVars := build(requireTags, "1 = 1")
	variables = append(variables, requireVars...)

	queryExclude, excludeVars := build(excludeTags, "1 != 1")
	variables = append(variables, excludeVars...)
...
}
...

func build(tags []*gqlmodel.InputTimeSpanTag, noop string) (string, []interface{}) {
	if len(tags) == 0 {
		return noop, nil
	}

	var have []string
	var haveParams []interface{}
	for _, tag := range tags {
		have = append(have, "(tstx.key = ? AND tstx.string_value = ?)")
		haveParams = append(haveParams, tag.Key, tag.Value)
	}

	return strings.Join(have, " OR "), haveParams
}

It results in this behaviour
image
image

@jmattheis
Copy link
Member

FYI: This week I'm pretty busy, I'll probably find some time around 22-26 December for testing the changes.

Copy link
Member

@jmattheis jmattheis left a comment

Choose a reason for hiding this comment

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

Awesome, thanks! Will create a release around new year.

@jmattheis jmattheis marked this pull request as ready for review December 26, 2025 14:45
@jmattheis jmattheis merged commit 0ada01a into traggo:master Dec 26, 2025
1 check passed
@jmattheis
Copy link
Member

Released with 0.8.1

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.

By-Value Filtering of Tags in Dashboard Entries

2 participants