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.

Sign up for GitHub

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

Jump to bottom

Should keep grafana log dir consistent with other components #999

Closed
lucklove opened this issue Dec 18, 2020 · 5 comments
Closed

Should keep grafana log dir consistent with other components #999

lucklove opened this issue Dec 18, 2020 · 5 comments
Labels
difficulty/easy Indicates that a issue is easy to resolve. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. type/bug Categorizes issue as related to a bug.

Comments

@lucklove
Copy link
Member

By default, the component will store logs in {.DeployDir}/log dir, however, grafana is an exception: https://github.com/pingcap/tiup/blob/master/templates/config/grafana.ini.tpl#L20

We should correct this by use logs = {{.DeployDir}}/log in the grafana.ini.tpl
@lucklove lucklove added the type/bug Categorizes issue as related to a bug. label Dec 18, 2020
@lucklove
Copy link
Member Author

/difficulty easy

@ti-chi-bot ti-chi-bot added the difficulty/easy Indicates that a issue is easy to resolve. label Dec 18, 2020
@rkazak
Copy link
Contributor

rkazak commented Mar 27, 2021

Has this already been fixed?

I see in the file,

tiup/embed/templates/config/grafana.ini.tpl

Line 20 in 1225c01

logs = {{.DeployDir}}/logs

However under the process I see

grafana-s 3713 rkazak    3w      REG                1,4    737622          8639198643 /Users/rkazak/.tiup/data/SStQvD7/grafana/log/grafana.log

So is the issue here that logs in the template file is not being honored and we end up with log as the directory on the active system?

@lucklove
Copy link
Member Author

lucklove commented Mar 29, 2021 •
edited
Loading

tiup/embed/templates/config/grafana.ini.tpl

Not yet, should be logs = {{.DeployDir}}/log instead of logs = {{.DeployDir}}/logs, because TiUP will create {{.DeployDir}}/log dir for each component. If it's {{.DeployDir}}/logs in grafana config, grafana will use it's own log dir {{.DeployDir}}/logs. It's not consistent with other components.

@lucklove lucklove added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Mar 29, 2021
@lucklove
Copy link
Member Author

Do you have interest to fix this?

@rkazak
Copy link
Contributor

rkazak commented Apr 2, 2021

Sure I can take a look at this. What threw me was that according to the 'lsof' output it is as required? So did it get over-ridden somewhere along the way?

specifically in in test it was => /Users/rkazak/.tiup/data/SStQvD7/grafana/log/grafana.log

rkazak added a commit to rkazak/tiup that referenced this issue Apr 2, 2021
@rkazak
Update grafana.ini.tpl
cdb7e3d 
For issue pingcap#999 
Modified as suggested to ensure that grafana uses a '.../log' directory.
@rkazak rkazak mentioned this issue Apr 2, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
difficulty/easy Indicates that a issue is easy to resolve. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. type/bug Categorizes issue as related to a bug.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@AstroProfundis @rkazak @lucklove @ti-chi-bot