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(tracex): generate archival from single transaction-done event #780

Merged
merged 1 commit into from
Jun 1, 2022

Conversation

bassosimone
Copy link
Contributor

@bassosimone bassosimone commented Jun 1, 2022

Tracex contained some fragile code that assembled HTTP measurements
from scattered events, which worked because we were sure we were
performing a single measurement at any given time.

This diff restructures the code to emit a transaction-start and a
transaction-done events only. We have basically removed all the other
HTTP events (which we were not using). We kept the transaction-start,
though, because it may be useful to see it when reading events. In
any case, what matters here is that we're now using the transaction-done
event alone to generate the archival HTTP measurement.

Hence, the original issue has been addressed. We will possibly
do more refactoring in the future, but for now this seems sufficient.

Part of ooni/probe#2121

Tracex contained some fragile code that assembled HTTP measurements
from scattered events, which worked because we were sure we were
performing a single measurement at any given time.

This diff restructures the code to emit a transaction-start and a
transaction-done events only. We have basically removed all the other
events (which we were not using). We kept the transaction-start
though, because it may be useful to see it when reading events. In
any case, what matters here is that we're now using the transaction-done
event aline to generate the archival HTTP measurement.

Hence, the original issue has been addressed. We will possibly
do more refactoring in the future, but for now this seems sufficient.

Part of ooni/probe#2121
@bassosimone bassosimone requested a review from hellais as a code owner June 1, 2022 17:16
@bassosimone bassosimone merged commit 6212daa into master Jun 1, 2022
@bassosimone bassosimone deleted the issue/2121 branch June 1, 2022 17:27
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.

1 participant