OpenLiberty / docs

See Open Liberty documentation on https://openliberty.io/docs/
https://openliberty.io/docs/
Other
12 stars 46 forks source link

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

Open scottkurz opened 1 year ago

scottkurz commented 1 year ago

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 commented 1 year ago

Chat conversation with Scott, Don & David

Ramkumar K Hi Scott, Don I will be taking up the following issue for documentation update: https://github.com/OpenLiberty/docs/issues/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: https://github.com/OpenLiberty/open-liberty/issues/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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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_LOGDIR 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 commented 1 year ago

@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?

NottyCode commented 1 year ago

@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 commented 1 year ago

@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?

ramkumar-k-9286 commented 1 year ago

Moving this issue into Icebox.

Do not have enough information at this time.

Regards, Ramkumar