https://github.com/apache/incubator-trafficcontrol/pull/1768 https://github.com/apache/incubator-trafficcontrol/pull/1769
@oren Do those solve your use case? On Sun, Jan 7, 2018 at 1:36 AM, Oren Shemesh <[email protected]> wrote: > Thank you Robert :-) > > On Thu, Jan 4, 2018 at 5:49 PM, Robert Butts <[email protected]> > wrote: > > > Eh, you're right, a Profile is the better place, since groups of servers > > will likely be configured with the same type. Much as it pains me to put > > more things in Parameters. > > > > And it can be reasonably safe, if we assume a missing Parameter implies > the > > existing Cache Type. (As safe as any Profile/Parameter can be; it goes > > without saying that typos will break things at runtime, the best we can > do > > is log errors in the Monitor if an unknown Stats Type Parameter value is > > found) > > > > I'd suggest naming the Parameter starting with `health.` (e.g. > > `health.polling.type`), which should make the existing Traffic Ops code > > automatically put it in `monitoring.json`. So then Traffic Ops code > doesn't > > need changed, just the Monitor. > > > > > > On Thu, Jan 4, 2018 at 6:01 AM, Oren Shemesh <[email protected]> wrote: > > > > > Robert, I see your valid point re the ability to automatically infer > the > > > stats "type". > > > So yes, given that the host in the stat is the origin, a single word > is a > > > valid domain, hence it is not possible to auto-detect it. > > > > > > I believe that adding a parameter to the server profile is the way to > do > > > this. > > > > > > Thanks ! > > > > > > Oren. > > > > > > On Tue, Jan 2, 2018 at 6:42 PM, Robert Butts <[email protected] > > > > > wrote: > > > > > > > Erm, @neuman beat me to it. > > > > > > > > On Tue, Jan 2, 2018 at 9:42 AM, Robert Butts < > [email protected] > > > > > > > wrote: > > > > > > > > > >can you send a pointer to any documentation/text available > regarding > > > > > 'Stats over HTTP' ? > > > > > > > > > > https://docs.trafficserver.apache.org/en/latest/admin-guide/ > > > > > plugins/stats_over_http.en.html#admin-plugins-stats-over-http > > > > > > > > > > https://github.com/apache/trafficserver/tree/c7cfa081ccc5c0b > > > > > 875a8ee665cde3399c48abd5c/plugins/stats_over_http > > > > > > > > > > It's essentially the "official" version of "Astats". In a nutshell, > > > > Astats > > > > > is a fork of that, and adds the "System" stats key. It's possible > to > > > use > > > > > the unmodified `stats_over_http`, while extending it to add the > > > "System" > > > > > stats Traffic Control needs. Which we're looking into doing, rather > > > than > > > > > maintaining a completely separate fork. But, as above, it may be > > > > difficult > > > > > to extend either to have the DS Name. Hopefully I'm wrong. > > > > > > > > > > > > > > > On Tue, Jan 2, 2018 at 9:35 AM, Robert Butts < > > [email protected] > > > > > > > > > wrote: > > > > > > > > > >> >I would like to avoid adding additional configurations to a > system, > > > in > > > > >> cases where the system can auto-detect the proper configuration. > > > > >> > > > > >> But the system can't, in this case. Detecting a single field as > > "must > > > be > > > > >> the DS Name" makes single-domain hosts impossible (such as > > > `localhost`). > > > > >> Single-domain names are perfectly valid within an internal > network, > > we > > > > >> shouldn't make that configuration impossible. > > > > >> > > > > >> I agree, minimizing configuration is ideal, but in this case, it > > isn't > > > > >> possible without making certain things mysteriously impossible; > and > > > > that's > > > > >> going to be a very confusing bug for someone to track down, if > they > > > > need it. > > > > >> > > > > >> Moreover, that configuration does exist, whether we try to > magically > > > > >> detect it or not. It is a property of the cache. And that's what > > > Traffic > > > > >> Ops is for -- configuration data about the cache should be stored > in > > > TO. > > > > >> Not having it there is going to make Operations' job more > difficult, > > > > >> because now they can't easily see what type of Stats a cache is > > > serving. > > > > >> For example, what if there's a bug and the Monitor is inferring > the > > > > wrong > > > > >> type? Now it's a nightmare for Ops and Dev to track down. > > > > >> > > > > >> I support what you want to accomplish, supporting multiple DSes > with > > > the > > > > >> same regex; and it's expensive to parse the domain anyway, it's > > ideal > > > to > > > > >> have the DS name in the stat. But unfortunately, it isn't possible > > to > > > > >> automatically infer without making certain configurations > > impossible, > > > > and > > > > >> failing to store the Stats type configuration (which exists, > whether > > > we > > > > >> store the data or not) in TO is going to lead to pain. > > > > >> > > > > >> >I do think it would be nice to have Traffic Monitor support some > > sort > > > > of > > > > >> canonical stats interface > > > > >> > > > > >> I have mixed feelings. On one hand, it'd be simpler, if there were > > > "One > > > > >> Right Way" with all the things we want/need. But on the other > hand, > > > that > > > > >> would make it painful to use Caches with Traffic Control which > don't > > > > have > > > > >> extensible stats. It'd mean users would have to set up an > > intermediary > > > > >> service (for every single Cache) to transform the Stats, for Cache > > > > >> applications which aren't sufficiently configurable. > > > > >> > > > > >> Maybe it'd be better make the Monitor able to easily be extended, > > > > similar > > > > >> to adding an endpoint in Golang TO, simply by adding a function > in a > > > new > > > > >> file, and adding that func to a "Server Cache Type" map? That > > solution > > > > >> seems fairly easy, and much easier for users to add custom stats > > than > > > > >> having to write and deploy an intermediary service. It will also > be > > > > safe, > > > > >> if we make the TO database field non-nullable and default to the > > > current > > > > >> type. > > > > >> > > > > >> > > > > >> Also be aware @orens that raw stats are part of the > > > /publish/CacheStats > > > > >> API in Traffic Monitor. Changing the Stats format will > fundamentally > > > > change > > > > >> that API endpoint (unless you also write the Monitor code to > rebuild > > > the > > > > >> domain, as inserted into the `Result.Astats.Ats` and > > > `ResultStatHistory` > > > > >> objects). I'm not sure if anything currently uses that (Router? > > > Traffic > > > > >> Stats?), but anything that does will break. Just something to be > > aware > > > > of. > > > > >> > > > > >> > > > > >> On Sun, Dec 31, 2017 at 1:35 AM, Oren Shemesh <[email protected]> > > > wrote: > > > > >> > > > > >>> Thanks Ryan, Robert and Dave for your responses. > > > > >>> > > > > >>> Dave, can you send a pointer to any documentation/text available > > > > >>> regarding > > > > >>> 'Stats over HTTP' ? > > > > >>> > > > > >>> I believe I can consider the overall response to the change I am > > > > >>> proposing > > > > >>> as positive :-) > > > > >>> > > > > >>> Robert, from my experience I would like to avoid adding > additional > > > > >>> configurations to a system, in cases where the system can > > auto-detect > > > > the > > > > >>> proper configuration. > > > > >>> Any configuration added is another chance for someone making a > > > mistake. > > > > >>> > > > > >>> With the proposal I have made, so configuration is needed: TM > > > > >>> auto-detects > > > > >>> the type of astats information (DS name / entire DS domain) based > > on > > > a > > > > >>> single 'if' statement which queries a single scalar value. > > > > >>> This also means that, assuming my proposed code change is valid, > > > there > > > > >>> can > > > > >>> be no performance issues. > > > > >>> > > > > >>> Also, I do not know what 'Stats over HTTP' is, but I hope it > keeps > > > the > > > > >>> job > > > > >>> of classifying Client HTTP transactions to a DS in the cache (ATS > > in > > > > our > > > > >>> case), avoiding the need to re-classify it in TM. > > > > >>> > > > > >>> (Note: In my mind, the fact that TM currently does this job, > based > > > on a > > > > >>> *single* hostName regex per DS, makes the entire feature of > having > > > > >>> multiple > > > > >>> regexps per DS very limited in use. Which is a shame, based on > the > > > > effort > > > > >>> invested in the DB model, TO API, and TR code). > > > > >>> > > > > >>> Thanks a lot guys ! > > > > >>> > > > > >>> > > > > >>> On Sat, Dec 30, 2017 at 7:33 PM, Dave Neuman <[email protected]> > > > > wrote: > > > > >>> > > > > >>> > Hi Oren, > > > > >>> > Sorry for the slow response, I think what you are proposing > > sounds > > > > >>> > reasonable, so +1. I would, however, like to make sure that we > > > > aren't > > > > >>> > affecting the performance of TM too much in terms of both CPU > > usage > > > > >>> (with > > > > >>> > 1000+ caches) and time to poll all caches/aggregate statistics. > > > > >>> > > > > > >>> > Additionally, I don't think we should be adding new > functionality > > > to > > > > >>> astats > > > > >>> > at all. The plan is to move towards the Stats Over HTTP plugin > in > > > the > > > > >>> > future, so we should focus on making Traffic Monitor do what we > > > need > > > > >>> it to > > > > >>> > do now, and add new functionality to Stats over HTTP if it > > doesn't > > > > >>> > currently exist. I do think it would be nice to have Traffic > > > Monitor > > > > >>> > support some sort of canonical stats interface instead of > > > > >>> > astats/stats_over_http, and I am sure there are several other > > > > features > > > > >>> > would be nice to have as well, but I don't think we should be > > > > muddying > > > > >>> the > > > > >>> > waters with that right now. > > > > >>> > > > > > >>> > > > > > >>> > On Wed, Dec 27, 2017 at 12:24 PM, Robert Butts < > > > > >>> [email protected]> > > > > >>> > wrote: > > > > >>> > > > > > >>> > > I'd be +1 if we extend Astats/stats_over_http to have the DS > > > name. > > > > >>> > > > > > > >>> > > It's easy to extend the Monitor to get the name from a field, > > if > > > > it's > > > > >>> > > available. And it should be faster than the current method. > > But, > > > > >>> Astats > > > > >>> > > doesn't currently support it, and in fact, I don't believe > ATS > > > has > > > > a > > > > >>> good > > > > >>> > > way to associate an identifier (DS name) with a Remap Rule. > Can > > > > >>> anyone > > > > >>> > > confirm? > > > > >>> > > > > > > >>> > > That said, if we're extending `stats_over_http`, I'd really > > like > > > to > > > > >>> see > > > > >>> > it > > > > >>> > > extended to serve CSV (ideally with the DS name in a field, > as > > > > >>> above), > > > > >>> > when > > > > >>> > > the request has an `Accept: text/csv` header. Astats only > has a > > > > >>> single > > > > >>> > > level of JSON, and they're all numbers. So there's no reason > to > > > use > > > > >>> JSON > > > > >>> > > over CSV, and CSV is drastically simpler and faster to parse. > > > > >>> Especially > > > > >>> > if > > > > >>> > > we break each `.` component of the stat into its own CSV > field. > > > The > > > > >>> > Monitor > > > > >>> > > is CPU-bound, so there's a good chance we'd see a noticeable > > > > >>> performance > > > > >>> > > improvement, which translates to a tangible poll time > > > improvement. > > > > >>> > > > > > > >>> > > Also @orens if we can't reasonably extend ATS Astats, we can > > > still > > > > >>> > support > > > > >>> > > your use case for your own cache, by having a separate code > > path > > > in > > > > >>> the > > > > >>> > > Monitor for your "stats", and adding a "stats type" number or > > > enum > > > > >>> to the > > > > >>> > > Server table (or ideally a new table for "cache servers", > since > > > it > > > > >>> only > > > > >>> > > applies to caches). So, the Monitor would look at that field > > > > >>> (possibly > > > > >>> > > adding it to `monitoring.json` or `CRConfig.json`), to > > determine > > > > how > > > > >>> to > > > > >>> > > parse a given server's stats. > > > > >>> > > > > > > >>> > > We've discussed a "stat version" field anyway, for different > > > Astats > > > > >>> > > versions. For example, we're looking into combining the > System > > > > stats > > > > >>> into > > > > >>> > > the Ats key, and we'd also like to be able to experiment with > > > > >>> different > > > > >>> > > formats (CSV, Protobuf, etc). I think it's a good feature for > > the > > > > >>> CDN in > > > > >>> > > general, to make stat polling more flexible. > > > > >>> > > > > > > >>> > > > > > > >>> > > On Wed, Dec 27, 2017 at 12:21 PM, Durfey, Ryan < > > > > >>> [email protected]> > > > > >>> > > wrote: > > > > >>> > > > > > > >>> > > > Oren, > > > > >>> > > > > > > > >>> > > > This is a great conversation to have. I can’t speak to the > > best > > > > >>> > solution, > > > > >>> > > > but allowing our servers to have a single SSL cert that > could > > > be > > > > >>> shared > > > > >>> > > > across delivery services would save us a lot of time, > money, > > > and > > > > >>> > > > operational resources. From a product perspective this > would > > be > > > > >>> highly > > > > >>> > > > beneficial to the platform especially since all services > are > > > > >>> eventually > > > > >>> > > > moving to SSL. > > > > >>> > > > > > > > >>> > > > Key Benefits: > > > > >>> > > > > > > > >>> > > > 1. Instant Service Builds: All new services could use > SSL > > > > >>> > immediately, > > > > >>> > > > no 2-3 business day delays in ordering SSL certs. > > > > >>> > > > * This gets us a step closer to instant SSL service > > > builds > > > > >>> > without > > > > >>> > > > the need to implement something like “letsencrypt”. > > > > >>> > > > 2. Operations Load: Reducing the management of a > constant > > > flow > > > > >>> of > > > > >>> > > > orders and renewals for SSL certs would save operations > > > resources > > > > >>> and > > > > >>> > > time. > > > > >>> > > > 3. Cost Reduction: A reduction in SSL certs equates to > > > > >>> reduction in > > > > >>> > > > purchase and renewal costs. > > > > >>> > > > > > > > >>> > > > > > > > >>> > > > > > > > >>> > > > Ryan Durfey M | 303-524-5099 > > > > >>> > > > CDN Support (24x7): 866-405-2993 or > [email protected] > > > > <mailto > > > > >>> : > > > > >>> > > > [email protected]> > > > > >>> > > > > > > > >>> > > > > > > > >>> > > > From: Oren Shemesh <[email protected]> > > > > >>> > > > Reply-To: "[email protected]" < > > > > >>> > > > [email protected]> > > > > >>> > > > Date: Wednesday, December 27, 2017 at 2:49 AM > > > > >>> > > > To: "[email protected]" < > > > > >>> > > > [email protected]> > > > > >>> > > > Subject: Two different Delivery Services using the same > host > > > > >>> regexp: A > > > > >>> > > > suggested Traffic Monitor code change > > > > >>> > > > > > > > >>> > > > Hi TC Dev people, > > > > >>> > > > > > > > >>> > > > We would like to avoid having to issue a specific SSL > > > certificate > > > > >>> for > > > > >>> > > every > > > > >>> > > > DS, as is needed today. > > > > >>> > > > > > > > >>> > > > (Reminder: Every DS needs a separate certificate because > The > > > > >>> domains > > > > >>> > used > > > > >>> > > > by the DS are tr.<ds-name>.<cdn-domain> and > > > > >>> > > > <cache-name>.<ds-name>.<cdn-domain>, and wild-card certs > > > support > > > > >>> only > > > > >>> > a > > > > >>> > > > single *, so every cert is issued to > > *.<ds-name>.<cdn-comain>, > > > > >>> which > > > > >>> > > makes > > > > >>> > > > it DS-specific). > > > > >>> > > > > > > > >>> > > > When configuring two different DSes to use the same host > > > regexp, > > > > >>> and > > > > >>> > > > configuring additional path regexps to differentiate > between > > > > them, > > > > >>> it > > > > >>> > is > > > > >>> > > > possible to overcome this issue. > > > > >>> > > > > > > > >>> > > > For example: DS 'my-ds1' can have host regexp > .*\.my-ds\..* , > > > and > > > > >>> path > > > > >>> > > > regexp /ds1/.* (Both with order 0), and DS 'my-ds2' has the > > > same > > > > >>> host > > > > >>> > > > regexp and path regexp /ds2/.*. > > > > >>> > > > > > > > >>> > > > This makes TR redirect requests of the form > > > > >>> > > > tr.my-ds.<cdn-domain>/ds1/<whatever> to > > > > >>> > > > <cache-name>.my-ds.<cdn-domain>/ds1/<whatever>, do the > same > > > for > > > > >>> > > > tr.my-ds.<cdn-domain>/ds2/<whatever>, and respond with > '503 > > > > >>> service > > > > >>> > > > unavailable' to requests to tr.my-ds.<cdn-domain> which do > > not > > > > >>> start > > > > >>> > with > > > > >>> > > > /ds1/ or /ds2/. > > > > >>> > > > > > > > >>> > > > So with TR, So far so good. > > > > >>> > > > > > > > >>> > > > ATS can also differentiate between the DSes based on the > > path, > > > so > > > > >>> there > > > > >>> > > is > > > > >>> > > > no problem there as well. > > > > >>> > > > > > > > >>> > > > However, there is a problem with Traffic Monitor: > > > > >>> > > > > > > > >>> > > > Because astats sends per-DS info based on the origin name, > TM > > > > >>> requires > > > > >>> > > that > > > > >>> > > > every DS has a unique host regexp (See > > > > >>> > > > https://github.com/apache/incubator-trafficcontrol/blob/ > > > > >>> > > > 3e3a5f41d482a065f46ef287b347e66d7d205d82/traffic_monitor/ > > > > >>> > > > todata/todata.go#L234 > > > > >>> > > > ). > > > > >>> > > > This is so that TM can map origin names from astats back > to a > > > DS. > > > > >>> > > > > > > > >>> > > > So, I would like to make a small change to the TM code: > > > > >>> > > > > > > > >>> > > > If the astats info contains only a single host name, assume > > it > > > is > > > > >>> > already > > > > >>> > > > the DS name. > > > > >>> > > > > > > > >>> > > > (This would be implemented in <goog_1101383350> > > > > >>> > > > https://github.com/apache/incubator-trafficcontrol/blob/ > > > > >>> > > > 3e3a5f41d482a065f46ef287b347e66d7d205d82/traffic_monitor/ > > > > >>> > > > cache/cache.go#L419 > > > > >>> > > > : > > > > >>> > > > > > > > >>> > > > if len(statParts) == 2 { // [0] is DS name, [1] is statName > > > > >>> > > > ds = statParts[0] > > > > >>> > > > } > > > > >>> > > > else { > > > > >>> > > > ... existing code which uses statsParts[0:len(statsParts)- > 1] > > > > >>> > > > } > > > > >>> > > > > > > > >>> > > > Obviously, the ds should be checked against the list of all > > > DSes > > > > to > > > > >>> > make > > > > >>> > > > sure it is valid, I wrote this code line just to > demonstrate > > > the > > > > >>> idea). > > > > >>> > > > > > > > >>> > > > Of course, the astats plugin for ATS might not support this > > > today > > > > >>> (I am > > > > >>> > > not > > > > >>> > > > sure), but a different cache (Hint: Qwilt cache) has no > > problem > > > > >>> > reporting > > > > >>> > > > the astats information per DS name. > > > > >>> > > > If needed, the astats code could be improved to do that, as > > > well. > > > > >>> > > > > > > > >>> > > > I believe that this change, while being very small and > > > > unobtrusive, > > > > >>> > would > > > > >>> > > > make the information passed from caches to TM 'cache vendor > > > > >>> agnostic', > > > > >>> > > > while also eliminating the need to re-classify URLs into DS > > > > (Which > > > > >>> is > > > > >>> > > what > > > > >>> > > > happens today in TM), hence it is a good step in a good > > > > direction. > > > > >>> > > > > > > > >>> > > > Any comments / opinions, please ? > > > > >>> > > > > > > > >>> > > > -- > > > > >>> > > > > > > > >>> > > > *Oren Shemesh* > > > > >>> > > > Qwilt | Work: +972-72-2221637 <+972%2072-222-1637>| Mobile: > > > > >>> +972-50-2281168 <+972%2050-228-1168> | > > > > >>> > [email protected] > > > > >>> > > > <mailto:[email protected]> > > > > >>> > > > <[email protected]<mailto:[email protected]>> > > > > >>> > > > > > > > >>> > > > > > > > >>> > > > > > > >>> > > > > > >>> > > > > >>> > > > > >>> > > > > >>> -- > > > > >>> > > > > >>> *Oren Shemesh* > > > > >>> Qwilt | Work: +972-72-2221637| Mobile: +972-50-2281168 | > > > > [email protected] > > > > >>> <[email protected]> > > > > >>> > > > > >> > > > > >> > > > > > > > > > > > > > > > > > > > > > -- > > > > > > *Oren Shemesh* > > > Qwilt | Work: +972-72-2221637| Mobile: +972-50-2281168 | > [email protected] > > > > > > > > > -- > > *Oren Shemesh* > Qwilt | Work: +972-72-2221637| Mobile: +972-50-2281168 | [email protected] >
