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

fix(traceql): fix extract matcher regexes to work with regexp-type matchers #3641

Merged
merged 3 commits into from
May 8, 2024

Conversation

sd2k
Copy link
Contributor

@sd2k sd2k commented May 1, 2024

This improves the regexes used by autocomplete (and I think the search tags v2 endpoint?) so that they work with queries like { resource.service.name = "app" && resource.k8s.cluster.name=~"prod-.+" }.

Fixes #3635.

@CLAassistant
Copy link

CLAassistant commented May 1, 2024

CLA assistant check
All committers have signed the CLA.

@sd2k sd2k force-pushed the 3635-autocomplete-regex-issues branch from bfddfc8 to d22cbaa Compare May 1, 2024 21:24
@sd2k sd2k marked this pull request as ready for review May 1, 2024 21:25
@joe-elliott
Copy link
Member

unsure why the drone CI failed. I have restarted it.

can you sign the CLA and add a changelog entry?

@sd2k sd2k force-pushed the 3635-autocomplete-regex-issues branch from d22cbaa to d12fe56 Compare May 2, 2024 17:50
@sd2k
Copy link
Contributor Author

sd2k commented May 2, 2024

Hmm I've rebased on main just in case. Signed the CLA and added a changelog entry too 👍

@sd2k sd2k force-pushed the 3635-autocomplete-regex-issues branch from 31af1b1 to 1539758 Compare May 7, 2024 20:13
@mdisibio
Copy link
Contributor

mdisibio commented May 8, 2024

Drone is failing on the step to test the debian package because tempo isn't starting up. It doesn't seem like it captures the tempo logs. Unlikely to be caused by the changes in this PR, but digging into it.

@mdisibio
Copy link
Contributor

mdisibio commented May 8, 2024

I wasn't able to do exact reproduction of the drone steps because of systemd-in-docker woes, but was able to confirm that make release-snapshot builds the deb, it installs, and the included tempo binary starts ok with /usr/bin/tempo -config.file /etc/tempo/config.yml. Leaning towards bypassing the drone step and merging.

level=info ts=2024-05-08T12:36:44.545535434Z caller=main.go:118 msg="Starting Tempo" version="(version=2.4.0-SNAPSHOT-1539758b9, branch=3635-autocomplete-regex-issues, revision=1539758b9)"
level=info ts=2024-05-08T12:36:44.548547946Z caller=server.go:240 msg="server listening on addresses" http=[::]:3200 grpc=[::]:9095
...
level=info ts=2024-05-08T12:36:44.623156247Z caller=app.go:214 msg="Tempo started"

@mdisibio
Copy link
Contributor

mdisibio commented May 8, 2024

The drone issues were resolved in #3657 and all looks good. Thanks.

@mdisibio mdisibio merged commit 5b250a7 into main May 8, 2024
15 checks passed
@mdisibio mdisibio deleted the 3635-autocomplete-regex-issues branch May 8, 2024 16:38
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.

Autocomplete regex several issues
4 participants