Skip to content

Drop Vulture Loki dependency#509

Merged
joe-elliott merged 23 commits intografana:masterfrom
dgzlopes:refactor/drop-vulture-loki-dep
Mar 5, 2021
Merged

Drop Vulture Loki dependency#509
joe-elliott merged 23 commits intografana:masterfrom
dgzlopes:refactor/drop-vulture-loki-dep

Conversation

@dgzlopes
Copy link
Copy Markdown
Member

@dgzlopes dgzlopes commented Feb 6, 2021

What this PR does:
This PR refactors Tempo Vulture to remove the actual Loki dep.

Now, instead of searching for traces in Loki we:

  • Generate a new trace (with some random data/spans).
  • Push the trace to Tempo (in batches).
  • Query a "pushed" trace randomly.

Which issue(s) this PR fixes:
Fixes #374

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Copy link
Copy Markdown
Collaborator

@joe-elliott joe-elliott left a comment

Choose a reason for hiding this comment

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

Great progress!

I tried to comment on what I was looking for w/r to the random # seeding. If that's unclear please reach out.

Comment thread cmd/tempo-vulture/main.go Outdated
Comment thread cmd/tempo-vulture/main.go Outdated
Comment thread cmd/tempo-vulture/main.go
@dgzlopes dgzlopes force-pushed the refactor/drop-vulture-loki-dep branch from 6d43578 to 49d3335 Compare February 11, 2021 13:16
Comment thread cmd/tempo-vulture/main.go
Comment thread cmd/tempo-vulture/main.go Outdated
Copy link
Copy Markdown
Collaborator

@joe-elliott joe-elliott left a comment

Choose a reason for hiding this comment

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

one nit, then i'll merge.

Comment thread cmd/tempo-vulture/main.go Outdated
@dgzlopes dgzlopes changed the title [WIP] Drop Vulture Loki dependency Drop Vulture Loki dependency Feb 16, 2021
@dgzlopes dgzlopes marked this pull request as ready for review February 16, 2021 14:47
Copy link
Copy Markdown
Collaborator

@joe-elliott joe-elliott left a comment

Choose a reason for hiding this comment

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

this is really close. one last comment, but i can be persuaded otherwise.

Comment thread cmd/tempo-vulture/main.go Outdated
}
startTime := time.Now().Unix()
ticker := time.NewTicker(tempoBackoffDuration)
slot := int64(tempoBackoffDuration/time.Second) * 2
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

i'd like to drop this slot idea and just generate the directly on the timestamps. i think this will make things easier to replicate outside the vulture.

i also think we should split the read/write tickers. maybe we only want to generate a trace every 30 seconds, but execute a query every 5 seconds?

Copy link
Copy Markdown
Member Author

@dgzlopes dgzlopes Feb 28, 2021

Choose a reason for hiding this comment

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

i'd like to drop this slot idea and just generate the directly on the timestamps. i think this will make things easier to replicate outside the vulture.

I removed the slot thing but from my POV the result was not great... Maybe I misunderstood something...

But, I think your interval approach is great, so I refactored the code to replicate that behavior. Now it's much cleaner and works flawlessly. Hopefully, it makes some sense!

i also think we should split the read/write tickers. maybe we only want to generate a trace every 30 seconds, but execute a query every 5 seconds?

Done :)

@dgzlopes dgzlopes force-pushed the refactor/drop-vulture-loki-dep branch 2 times, most recently from 4d62e83 to 3896143 Compare March 1, 2021 20:12
dgzlopes added 19 commits March 4, 2021 17:26
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
dgzlopes added 4 commits March 4, 2021 17:34
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
@dgzlopes dgzlopes force-pushed the refactor/drop-vulture-loki-dep branch from 10fe89e to 10f4600 Compare March 4, 2021 16:36
@dgzlopes dgzlopes requested a review from joe-elliott March 4, 2021 17:08
@joe-elliott joe-elliott merged commit c8cad66 into grafana:master Mar 5, 2021
@dgzlopes dgzlopes deleted the refactor/drop-vulture-loki-dep branch March 5, 2021 18:56
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.

Drop Vulture's Loki Dependency

2 participants