Add TO Go cdns/routing - DO NOT MERGE

[rob05c]

Rewrites /cdns/routing to Go. Closes #3781

[asfgit]

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/1626/
Test PASSed.

[asfgit]

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/1644/
Test PASSed.

[asfgit]

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/1665/
Test FAILed.

[mitchell852]

retest this please

[asfgit]

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/1666/
Test PASSed.

[asfgit]

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/1762/
Test PASSed.

[dewrich]

We need to solve custom TO extensions or the ability to override for custom project's needs

[mitchell852]

yes, because TO_golang has no concept of to_extensions (at the moment), merging this PR would kill the perl to_extension that is currently being used by Comcast in production.

[asfgit]

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/2041/
Test PASSed.

[dewrich]

Please DO NOT MERGE this until Traffic Ops extension-like functionality gets resolved

[ocket8888]

This endpoint has already been re-written to Go, so I think this PR should be closed.

[rawlinp]

This endpoint has not been re-written to Go. Just because there is a route in routes.go does not mean that it's been rewritten. If you look closely the handler is proxyHandler, which is an explicit route to TO-Perl (as opposed to unmatched routes getting routed to TO-Perl by default).

		{1.1, http.MethodGet, `cdns/routing$`, handlerToFunc(proxyHandler), 0, NoAuth, []Middleware{}},

[ocket8888]

My bad

[asfgit]

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4137/
Test FAILed.

[mhoppa]

@rob05c What can we do to get this past DO NOT MERGE?

[rob05c]

@mhoppa We have a Plugins system in TO now. The "DO NOT MERGE" was added because Comcast has an non-open-source Perl TO Extension overriding this route. It's a Comcast thing, not an Open Source thing. Comcast has requested the Open Source project wait to implement this route, until Comcast has had time to rewrite its private Perl Extension into a Plugin.

[mhoppa]

Okay makes sense. I am concerned that in a 1 year and half no work has been done on it to rewrite it as this will soon be blocking our completion of our rewrite. We will need to get Comcast to prioritize that work.

Is the non-open-source Perl TO Extension overriding this route something that could be made open source? why did Comcast do that in the first place?

[rob05c]

It's because it uses an internal log data tool, which itself involves a large amount of infrastructure.
It's not that Comcast doesn't want to Open Source it, it's just that that infrastructure isn't really related to TC and doesn't make sense for the project.

[ocket8888]

Can this be safely merged once #4145 is merged?

[mitchell852]

Can this be safely merged once #4145 is merged?

I believe so because #4145 allows you to turn off certain Go routes if desired.

[rawlinp]

I question the need for our proprietary version in the first place. I think the issue was that we were adding a lot of TRs and didn't want to have to query 20ish TRs sequentially (because that's what the Perl implementation did) before returning the resulting data. If each query took a few seconds, we'd probably hit some timeouts around the 30 sec or 1 min mark.

TBH I think we can just make this Go implementation query all the TRs in parallel, then aggregate the responses. That should only take as much time as the slowest query to TR, and basically gets us the exact same results as our proprietary Perl plugin.

What do you guys think?

[mhoppa]

I question the need for our proprietary version in the first place. I think the issue was that we were adding a lot of TRs and didn't want to have to query 20ish TRs sequentially (because that's what the Perl implementation did) before returning the resulting data. If each query took a few seconds, we'd probably hit some timeouts around the 30 sec or 1 min mark.

TBH I think we can just make this Go implementation query all the TRs in parallel, then aggregate the responses. That should only take as much time as the slowest query to TR, and basically gets us the exact same results as our proprietary Perl plugin.

What do you guys think?

If that was the issue I would be +1 on that approach

[ocket8888]

That's fine with me, it certainly doesn't make any breaking changes to just do things in parallel.

[rob05c]

I'm unlikely to be implementing, so I'll defer to the people who will be.

[rawlinp]

I'm almost certain that was the issue. We weren't very far into the Go rewrite by the point that TO extension was written, and Perl doesn't do parallelism very easily. I think we were already using telegraf to periodically query the TR API and write it to influxdb, so we said "why not just add some continuous queries over that data and write a Perl TO extension to query that?"

At the end of the day, this API basically just gives you a pie-chart breakdown of routing methods (percentages of CZF, GEO, etc), and I don't really see a problem with just getting the data directly from TRs in parallel and aggregating them.

@rob05c should whoever works on parallelizing this just open a PR against your fork's branch?

[rob05c]

@rob05c should whoever works on parallelizing this just open a PR against your fork's branch?

Sure. Or it's not very big, they could just copy and paste the reusable parts of this, it doesn't matter to me if my name isn't on it, I know Git can be a pain. Whatever's easiest for the author.

[mitchell852]

Comcast has an non-open-source Perl TO Extension overriding this route.

which appears to be broken anyhow. I'd say we remove the wip label and get this merged.

[rawlinp]

If we merge as-is this API will take a very long time to return with large numbers of TRs since they're each polled sequentially -- that was the problem with the original Perl version. IMO we should parallelize it, so that the amount of time it takes doesn't scale with the number of TRs.

[mhoppa]

PR #4370 replaces this

[mitchell852]

closing as a new PR has been opened to replace this - #4370