Skip to content
This repository has been archived by the owner on Aug 23, 2023. It is now read-only.

Update Graphite tags models to include From and To fields #2013

Merged
merged 1 commit into from
Nov 12, 2021

Conversation

carrieedwards
Copy link
Contributor

@carrieedwards carrieedwards commented Nov 4, 2021

This PR updates the GraphiteTags and GraphiteAutoCompleteTags models to include a FromTo field. This will enable the tags query to include from and to fields. This is part of changes necessary for an issue in which no tags will appear in the UI if metrics have not been ingested in the last hour

Related PR:

@Dieterbe
Copy link
Contributor

Dieterbe commented Nov 5, 2021

Does graphite upstream use these parameters too?

Note that we keep a list of differences between metrictank and graphite at docs/graphite.md (this may need to be updated)
and also we keep our http api documentation in docs/http-api.md (will certainly need an update)

@robert-milan
Copy link
Contributor

robert-milan commented Nov 9, 2021

You will probably need to run go generate ./..., other than that I think it is fine.

As for updating the documentation, let me know if you need help on that.

@carrieedwards carrieedwards marked this pull request as ready for review November 9, 2021 18:58
@shanson7
Copy link
Collaborator

It seems like this adds the fields but they aren't used/respected in downstream code. Should they be?

@Dieterbe
Copy link
Contributor

this is for GEM, which uses the same models and expr library.
In metrictank we don't actually use the fields, so we don't need to really update docs.
This could get a bit confusing, but I think it's fine.

@Dieterbe
Copy link
Contributor

You will probably need to run go generate ./..., other than that I think it is fine.

No change. probably because this is external api (towards users), not the internal api between metrictank nodes which uses messagepack

@Dieterbe Dieterbe merged commit f1e39bc into master Nov 12, 2021
@Dieterbe Dieterbe deleted the cedwards/update-graphite-tags-models branch November 12, 2021 20:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants