Implement anonymizer's main program

[Ashmita152]

Which problem is this PR solving?

Implement anonymizer's main program, per #2556

Short description of the changes

• add option to specify query service gRPC address to load the trace
• add option to specify trace ID to load from gRPC query service
• add option to specify output directory to save the file
• add options for Anonymizer configuration parameters
• when saving anonymized trace to a file, it first must be converted to the UI model (see writer package)

Validation

• the trace should be loadable by Jaeger UI (Load JSON tab in the Search panel)

Validation step is not working because the difference in the expected and actual json.

https://gist.github.com/Ashmita152/e9b421819571098d3c9de965b6ae689c

I am unable to figure out why this difference is happening and what should be the right way to fix.

[yurishkuro]

What do you mean by "Validation step is not working because the difference in the expected and actual json"? Which validation?

[Ashmita152]

Hi @yurishkuro

Thank you so much for the review. I will fix them. By saying "validation step is not working", I meant I am unable to load the anonymised json from the json tab in the search panel due to the difference in the produced json. It keeps rotating while loading the json from the file. After manually modifying the json to the expected.json (https://gist.github.com/Ashmita152/e9b421819571098d3c9de965b6ae689c) it works as expected.

[yurishkuro]

It keeps rotating while loading the json from the file

I assume you're talking about the UI. Is there an error in the Javascript console?

[Ashmita152]

Hi @yurishkuro

I have updated the PR based on your feedbacks. Those were great suggestions. Hopefully I didn't miss any of those. It's ready for review again.

I assume you're talking about the UI. Is there an error in the Javascript console?

There is no error in javascript console. No error logs in the all-in-one docker image too.

Thank you once again for the help.

[Ashmita152]

Hi @yurishkuro

Thank you for the review again. Those definitely made the PR lot better. I have tried my best to fix those and it would be great to have your review again. Regards.

[codecov]

Codecov Report

Merging #2621 (854ad35) into master (4e7935b) will decrease coverage by 0.00%.
The diff coverage is 97.82%.

@@            Coverage Diff             @@
##           master    #2621      +/-   ##
==========================================
- Coverage   95.07%   95.06%   -0.01%     
==========================================
  Files         209      210       +1     
  Lines        9368     9410      +42     
==========================================
+ Hits         8907     8946      +39     
- Misses        385      388       +3     
  Partials       76       76              

Impacted Files	Coverage Δ
cmd/anonymizer/app/anonymizer/anonymizer.go	60.49% <80.00%> (-0.76%)	⬇️
cmd/anonymizer/app/flags.go	100.00% <100.00%> (ø)
cmd/query/app/server.go	88.52% <0.00%> (-1.64%)	⬇️
...lugin/sampling/strategystore/adaptive/processor.go	99.07% <0.00%> (-0.93%)	⬇️
plugin/storage/badger/spanstore/reader.go	96.08% <0.00%> (+0.71%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4e7935b...854ad35. Read the comment docs.

[Ashmita152]

Hi @yurishkuro

Thank you for the feedbacks. I tried my best to update the PR based on my understanding. May I ask for your review again please. Regards.

[yurishkuro]

you're missing things like w.capturedFile.WriteString("\n]\n"). See L132 - that code should be moved into Close.

[yurishkuro]

how did you test it? are you able to load the resulting fie in the UI?

[Ashmita152]

Hi @yurishkuro

This is the output from a sample run: https://gist.github.com/Ashmita152/68904c2ffe2dc40bf17984ffc981cace
It doesn't load in the UI since the json isn't as expected I guess, no errors in js console: https://gist.github.com/Ashmita152/e9b421819571098d3c9de965b6ae689c
I am unable to figure out the reason about this inconsistency but my guess is that UI accepts a complete trace json while we are uploading one particular span json.

I have a question: why do we write span by span while writing to file, shouldn't we be first constructing a trace out of those spans and then write to file.

[yurishkuro]

ok, I see the confusion. The original purpose of the Writer was to use it while tailing a long stream of spans (in Kafka) to share them with researchers. It does not have the notion of a trace because spans are not ordered/grouped by trace ID in a stream. We had another internal binary that used these packages as a library.

For the purpose of this ticket, we know that we're dealing with a single trace. However, the Writer does not have the logic to write that trace in exactly the UI format, so you may need to add that. You can download a sample trace from the UI (via JSON button in the trace view) to see where the formatting is different.

[Ashmita152]

I see, that clears my doubt. Sure I will work on implementing that in writer. Do we need to keep the functionality backward compatible ie. allowing current tailing stream of spans too ?

[yurishkuro]

yes, please keep backward compatibility.

[Ashmita152]

I am confused about how to implement the logic to write the trace in the UI format. To me, it seems like we can't use WriteSpan function. I am thinking to do:

• Implement a WriteTrace function in writer.go
• Change QueryTrace in query.go which is now returning []model.Span. I am thinking to modify it to return model.Trace.
• In main.go instead of calling writer.writeSpan, call writer.writerTrace.

I would like to get your opinion before start implementing this way.

[yurishkuro]

yeah, the writer is not exactly compatible with saving a single trace. Let's merge this PR, since it achieved the basic anonymization of a trace, just doesn't output UI-compatible file, which can be added separately.

TBH, our internal program worked differently, in two stages - first we'd run it on a stream extracting lots of spans into an anonymized file. Then we had a separate command in the binary that would take a trace ID, scan the previously save file, pick the spans for the given trace ID, and output just that trace in the UI format. It would be weird to do it the same way here because the first run is already taking a trace ID as input (unlike our case where input was undifferentiated stream of spans from many traces). However, we could still implement it the same way, just invoke the second step automatically on the file already generated by your current code.

Let me do it as a separate PR.

[yurishkuro]

#2629