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

Add Multiple custom access logs and Header field for ATS #502

Closed
wants to merge 1 commit into from

Conversation

weifensh
Copy link
Contributor

As discussed in the dev maillist:
Currently TO supports only one custom access log file by configuring the "LogFormat" and "LogObject" parameters for "logs_xml.config". We have a requirement to support multiple custom access log files. As ATS does support this, we are planning to extend "LogFormat"&"LogObject" on TO, and add extra 9 groups (9 should be enough) of parameters like: "LogFormat1"&"LogObject1", "LogFormat2"&"LogObject2" ... "LogFormat9"&"LogObject9". Only when "LogFormatN"&"LogObjectN" are configured, they will appear in "logs_xml.config"; otherwise the file does not change, so the current behavior does not change either.

@asfgit
Copy link
Contributor

asfgit commented Aug 6, 2017

Can one of the admins verify this patch?

1 similar comment
@asfgit
Copy link
Contributor

asfgit commented Aug 14, 2017

Can one of the admins verify this patch?

@asfgit
Copy link
Contributor

asfgit commented Aug 15, 2017

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

@mitchell852 mitchell852 added Traffic Ops related to Traffic Ops Traffic Ops ORT *DEPRECATED* related to the traffic_ops_ort.pl script labels Oct 5, 2017
@dg4prez
Copy link
Contributor

dg4prez commented Oct 18, 2017

The configfiles.pm in UI is deprecated. These changes need to be made against the traffic_ops/app/lib/API/Configs/ApacheTrafficServer.pm file. If you can do that I will test and merge them.

@dg4prez dg4prez self-assigned this Oct 18, 2017
@weifensh
Copy link
Contributor Author

@dg4prez changes made on ApacheTrafficServer.pm as well

@smalenfant
Copy link
Contributor

@weifensh I installed this today in my lab. Can you give a little more explanation on how this should work? I removed LogFormat and created a LogFormat1 and LogFormat2. This resulted with my file having the following :

<LogFormat>
  <Name = ""/>
  <Format = ""/>
</LogFormat>
<LogObject>
  <Format = ""/>
  <Filename = ""/>
  <RollingEnabled = />
  <RollingIntervalSec = />
  <RollingOffsetHr = />
  <RollingSizeMb = />
</LogObject>

@smalenfant
Copy link
Contributor

@weifensh Here's an example of the parameters I imported. https://gist.github.com/smalenfant/6eb39724b9ddea55def4e99c5803733b

@smalenfant
Copy link
Contributor

smalenfant commented Oct 26, 2017

weifensh Apparently I needed the old ConfigFiles.pm as well...

@dg4prez Are both ConfigFiles.pm and ApacheTrafficServer.pm needed even with new ORT and API end point?

@dg4prez
Copy link
Contributor

dg4prez commented Oct 26, 2017 via email

@weifensh
Copy link
Contributor Author

@smalenfant the original "LogFormat" and "LogObject" could still be kept and still works. And new "LogFormatN" and "LogObjectN" could be added, each "LogObjectN.Format" may be assigned with the value of "LogFormatN.Name" so that they are linked.

@smalenfant
Copy link
Contributor

@weifensh Could you rebase so I can merge this change? Thanks. It's been tested and working great here.

@weifensh
Copy link
Contributor Author

@smalenfant Rebased, could you check if it is mergeable now?

@asfgit
Copy link
Contributor

asfgit commented Nov 21, 2017

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

@asfgit
Copy link
Contributor

asfgit commented Nov 21, 2017

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

@smalenfant
Copy link
Contributor

@weifensh Still got problem merging (through github). We might need some assistance on how to fix this.

@smalenfant
Copy link
Contributor

@weifensh I followed instructions here for rebase -i. Can you try the same?

https://www.digitalocean.com/community/tutorials/how-to-rebase-and-update-a-pull-request

basically, those following commands

git merge-base <branch> master
git rebase -i <commit_hash>
squash your commits
force-push

Hopefully that can do it. It will bring your commit as a single one on the top to the tree.

@asfgit
Copy link
Contributor

asfgit commented Nov 29, 2017

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

@weifensh
Copy link
Contributor Author

Thanks @smalenfant Haven't managed to rebase it successfully, so just opened a new PR:
#1599

@weifensh weifensh closed this Nov 29, 2017
@asfgit
Copy link
Contributor

asfgit commented Nov 29, 2017

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Traffic Ops ORT *DEPRECATED* related to the traffic_ops_ort.pl script Traffic Ops related to Traffic Ops
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants