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

The behavior of the server shell scripts in unsetting LOG_DIR env var is unexplained #6391

Open
scottkurz opened this issue Mar 3, 2023 · 8 comments

Comments

@scottkurz
Copy link
Member

On the one hand we do a fine job explaining how the LOG_DIR env var will configure the location of the Liberty logs, (see: https://openliberty.io/docs/latest/log-trace-configuration.html#settings).

What we don't explain is that a LOG_DIR env var supplied in server.env can not be referenced as an environment variable. It is unset in https://github.com/OpenLiberty/open-liberty/blob/d4d2cad32f282fa48b0f9b8db36a234f32a96fa5/dev/com.ibm.ws.kernel.boot.ws-server/publish/bin/server#L580-L588 and the values are transferred to an X_LOG_DIR.

So if a user wants to use/reference their LOG_DIR value in some other context, they don't have an easy way of doing so.

Of course, they could consider using another approach like the bootstrap.properties in some use cases. But in this use case from StackOverflow, using SpringBoot application properties, this wouldn't be an option.

Is it doc'd that they could use X_LOG_DIR in this fashion?

Should this be explained or rationalized briefly?

@ramkumar-k-9286
Copy link
Contributor

Chat conversation with Scott, Don & David

Ramkumar K
Hi Scott, Don
I will be taking up the following issue for documentation update: #6391
With regard to this issue, could you provide me with some details on the change that needs to be made to the document?

Scott Kurz
It'd be best if Don or someone on the team could validate...
It seems we made the decision that you couldn't reference the LOG_DIR env var in config... I'm imagining somewhere along the line someone stumbled upon some weird use case, given the primordial nature of logging, etc.
Should we just a sentence somewhere mentioning this?
We just had some confusion in StackOverflow https://stackoverflow.com/questions/75608178/spring-boot-on-open-liberty-illegalargumentexception-could-not-resolve-place/75622301 and it looks even internally we're confused about it: OpenLiberty/open-liberty#7461

I guess we could go on to mention X_LOG_DIR... but guessing we don't want to do that

Ramkumar K
Don, what do you think about this?:point_up_2:How should we proceed with documenting this?

Don Bourne
I don't know the history of why LOG_DIR is treated in this odd/special way. I'd suggest asking, on #was-liberty to try to clarify why it's the case (and rule out the possibility this is just a bug). If we are going to document something about it, the workaround described at https://stackoverflow.com/questions/75608178/spring-boot-on-open-liberty-illegalargumentexception-could-not-resolve-place/75622301 sounds promising, but I think we need to better understand why LOG_DIR is special before we do that.

Scott Kurz
Hmm... I tried in https://ibm-cloud.slack.com/archives/C31DW78RG/p1677778968369859 but didn't get any response. Maybe someone else could take a shot at it?

Don Bourne
was the "X_LOG_DIR" thing added long enough ago that there's no useful git history for why that changed?

looks like that was part of "initial commit" 6 years ago

unless someone on kernel team knows the rationale, we may just have to take it as "the way it is" and document what you (
@skurz
) came up with in your stackoverflow answer

Scott Kurz
Yeah ... there's no history even in GHE

Don Bourne
maybe we just add a footnote to the default env vars table at https://openliberty.io/docs/latest/reference/default-environment-variables.html to explain that LOG_DIR isn't available in the server process, but that you can create your own copy of it as explained...?

@ramkumar-k-9286
Copy link
Contributor

ramkumar-k-9286 commented Apr 14, 2023

While Open Liberty provides the LOG_DIR environment variable to configure the location of logs, it is not directly accessible as an environment variable within the server process. Instead, the value of the LOG_DIR environment variable is transferred to an X_LOG_DIR variable, which should be used when referencing the log directory in other contexts, such as configuration files.

What I understood from the conversation - please correct me if I'm wrong.

@scottkurz
Copy link
Member Author

When I see X_LOG_DIR that suggests to me that this was specifically intended not to be an external.

So my suggestion is that the user creates their own env_var to both set LOG_DIR and make a "copy" for other config rather than documenting this.

@ramkumar-k-9286
Copy link
Contributor

While Open Liberty provides the LOG_DIR environment variable to configure the location of logs, it is not directly accessible as an environment variable within the server process. Instead, the value of the LOG_DIR environment variable is transferred to an X_LOG_DIR variable, which should be used when referencing the log directory in other contexts, such as configuration files.

We could replace the mention of the X_LOG_DIR variable with an instruction for the user to create their own variable.

Could you suggest some language for this? @scottkurz @donbourne

@scottkurz
Copy link
Member Author

@NottyCode any chance you know the history here? Why we unset LOG_DIR env var and replace it with X_LOG_DIR for the running process?

@dmuelle dmuelle modified the milestones: 23.0.0.4, 23.0.0.5 May 1, 2023
@NottyCode
Copy link
Member

@scottkurz I don't know why we do this, but I don't really want to document X_LOG_DIR, it feels very much like an internal to me. Why do you think we need to document X_LOG_DIR or LOG_DIR exactly?

@scottkurz
Copy link
Member Author

@scottkurz I don't know why we do this, but I don't really want to document X_LOG_DIR, it feels very much like an internal to me. Why do you think we need to document X_LOG_DIR or LOG_DIR exactly?

@NottyCode I agree we shouldn't document X_LOG_DIR.

However, I think it's reasonable and useful for a user to expect they can access the LOG_DIR from their applications, and surprising to find out that we unset and wipe clean this variable, (it was surprising to the StackOverflow OP).

If there's nothing helpful we can say here perhaps the status quo is even better than making a change.

Perhaps I'm suggesting some sort of rationalization/explanation for why this is the case: "because logging is special we unset the LOG_DIR env var"??? I don't know... any ideas?

@dmuelle dmuelle modified the milestones: 23.0.0.5, 23.0.0.6 Jun 1, 2023
@ramkumar-k-9286 ramkumar-k-9286 removed this from the 23.0.0.6 milestone Jun 9, 2023
@ramkumar-k-9286
Copy link
Contributor

Moving this issue into Icebox.

Do not have enough information at this time.

Regards,
Ramkumar

@ramkumar-k-9286 ramkumar-k-9286 removed their assignment Jun 9, 2023
@dmuelle dmuelle removed the doc bug Something isn't working. label Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants