Confusing, if not downright wrong, warning message

[metacosm]

Describe the bug

I run my app in dev mode. I change a build time property in application.properties and the application properly restarts but I get the following warning in the console:

2022-02-10 09:57:08,244 WARN  [io.qua.run.con.ConfigRecorder] (Quarkus Main Thread) Build time property cannot be changed at runtime:
 - quarkus.operator-sdk.crd.apply was 'true' at build time and is now 'false'

What is this message trying to tell me? What is the state of my property, i.e. is it true or is it false?

Expected behavior

A clear message as to what happened.

Actual behavior

The currently ambiguous or wrong message.

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

11

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.7.0.Final

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

[metacosm]

Maybe better suited as a discussion? Though I'd argue that this is a bug in the current implementation because it conveys confusing information.

[maxandersen]

Is there a formulation that you think would be better?

maybe something like

Build time property cannot be changed at runtime:
 - quarkus.operator-sdk.crd.apply was requested to be 'false', but it is buildtime fixed to 'true'

@radcortez @geoand

[metacosm]

Well, can you tell me what is the current state of the property based on the warning message alone? If you can't then the message needs to be changed to something else, imo. Also, do you think that the message means that property was actually changed or not?

[maxandersen]

yes its true but I agree its not clear - thus I asked if there is a formulation that would been better and I gave my attempt.
Does that sound better or you have suggestion on how to make it clearer?

[gsmet]

/cc @radcortez

[metacosm]

I purposefully didn't detail what I did so that it wouldn't influence how the warning is interpreted in the absence of external information.

What I did was changed, while dev mode is running, the property from false to true. So why is the warning even talking about it being requested to be false? If the property value is properly changed to true, why do we even have a warning? What purpose does it serve (apart from confusing users 😄)?

To me, the proper behavior here would be to not issue any warning at all because there is nothing that requests the value to be false apart from the default value specified in the configuration (unless there's a bug in my extension, that is 🤔 but, as far as I can tell, there's nothing that attempts to set the property value, the extension is just reading it).

Note also that while working on this I experienced a time where the property wasn't actually changed (i.e. the behavior associated with the value being true was not triggered) no matter how many force restart I did (via pressing s on the console). Restarting the dev mode solved the issue.

[geoand]

I personally think the message clear but I realize than I am not the right person to comment since I have seen the message too many times

[metacosm]

Again, how does it make sense when nothing requests the property to be false?

[geoand]

I am referring to the message itself. For me, the message is correct.

What triggers it is something different where there might be a bug... (but I haven't been able to reproduce the issue FWIW)

[metacosm]

When a message tells me a property is "now false", when it's actually the opposite, sorry, but I don't find that clear as it conflicts with the rest of the message.
I see the overall logic but the wording is very poor, imo.

And when you add on top of that the fact that nothing is supposed to request the property to be false, then you can see how this message can be quite confusing. I could understand the message if there was indeed something requesting the property to be false at runtime but this isn't the case.

[radcortez]

And when you add on top of that the fact that nothing is supposed to request the property to be false, then you can see how this message can be quite confusing. I could understand the message if there was indeed something requesting the property to be false at runtime but this isn't the case.

What happens here is that build time properties are recorded and then matched against changes in the current properties and if differences are found they are reported as a warning (the one you see).

Are you suggesting that such a warning should only happen when the property is actually used?

[metacosm]

And when you add on top of that the fact that nothing is supposed to request the property to be false, then you can see how this message can be quite confusing. I could understand the message if there was indeed something requesting the property to be false at runtime but this isn't the case.

What happens here is that build time properties are recorded and then matched against changes in the current properties and if differences are found they are reported as a warning (the one you see).

I'm not sure I understand. In my case, the only thing that changes is the application.properties that is set at build time. There is nothing (at least that I control) that requests the property to have a different value at runtime. So, in this specific case, the warning should not even be there because no one is trying to override the build time value.

Are you suggesting that such a warning should only happen when the property is actually used?

The property is used, that's not the issue. The warning should only happen if there is actually something that tries to override the value set at build time, which isn't the case here.

The warning should also be reworded to be less ambiguous to something similar to what Max suggested (bonus point if we could tell what is requesting the property to be changed).

[radcortez]

I'm not sure I understand. In my case, the only thing that changes is the application.properties that is set at build time. There is nothing (at least that I control) that requests the property to have a different value at runtime. So, in this specific case, the warning should not even be there because no one is trying to override the build time value.

If I understood correctly, you change the application.properties file, but the configuration property, in this case for quarkus.operator-sdk.crd.apply keeps the same value?

[metacosm]

If I understood correctly, you change the application.properties file, but the configuration property, in this case for quarkus.operator-sdk.crd.apply keeps the same value?

No: I change the value in application.properties to true and then I get the warning and then I get confused by what it means because nothing is supposed to try to change the value back to false at runtime. :)

[calohmn]

@radcortez Using Quarkus 2.7.3, I get this warning in the log output:

2022-04-05 07:44:44,245 WARN [io.qua.run.con.ConfigRecorder] (main) Build time property cannot be changed at runtime:
quarkus.opentelemetry.propagators is set to 'tracecontext,baggage' but it is build time fixed to 'jaeger,baggage,tracecontext'. [...]

The default value of the property is "tracecontext,baggage" (i.e. build time value here).
My runtime config (application.yaml in container) contains

quarkus:
  opentelemetry:
    propagators:
      - "jaeger"
      - "baggage"
      - "tracecontext"

So, it seems to me the values are logged the wrong way around - the value is actually not build time fixed to 'jaeger,baggage,tracecontext' here.

I would have expected a warning message like

failed to set "quarkus.opentelemetry.propagators" to 'jaeger,baggage,tracecontext' because it is build time fixed to 'tracecontext,baggage''.

[calohmn]

@radcortez Could you verify the printout of property values in the committed fix is done in the wrong way around?

I also think the log message should emphasize that the runtime value actually isn't getting set/applied - see my suggestion above.

[radcortez]

Let me have a look.