feat: Support organization MDC

[jeanpommier]

Solves #195

Resolving the user and organisation information requires to intervene at a late stage, much later than the logging MDCWebFilter class (after ResolveGeorchestraUserGlobalFilter). Resolving the objects would also imply a circular dependency if implemented in the logging module. So, it seems to make more sense to implement it in the main geOrchestra gateway code.

[groldan]

Awesome

[groldan]

I don't know what to do about the failing builds though, they're reporting Cache service responded with 422

[jeanpommier]

I don't know what to do about the failing builds though, they're reporting Cache service responded with 422

They're failing on all pending PRs, right ? I was concerned first, then concluded that the CI was currently a bit broken

[jeanpommier]

Awesome

My first Gateway PR ! Champagne !

[jeanpommier]

About Cache service responded with 422, I believe this is not specificly tied to this PR, but let's quickly discuss it.
I'm really no expert at this, but searching for this kind of issue, I stumbled upon actions/setup-python#1085.
Given how they solved it, I'd suggest we try to upgrade the setup-java action from v2 to v4 (current)

[jeanpommier]

Back to this PR, before thinking about merging it, I'd like your opinion guys about something:
on my filter, I'm adding several MDC:

• org: uuid, shortname and full name (if logging org is enabled)
• user: uuid, firstname, lastname (if logging for user is enabled), which complements the already set username.

Why so many MDCs ? Well, since the idea is to provide analytics dataviz, it will help having user-friendly labels available. I could get there some other way afterwards, but it's more complicated
Why the uuids ? Well, AFAIK, geOrchestra now allows for username and org shortname to change over time. Why will cause problem tracking them. I'd rather rely on the objects' uuid for that

And now, here is my question: does it seem OK for you to have just one toggle config (user:true/false, org: true/false) for this bunch of MDCs, or do you think we should have more granular control around which ones we want (e.g. configure them one by one) ?

[jeanpommier]

About Cache service responded with 422, I believe this is not specificly tied to this PR, but let's quickly discuss it. I'm really no expert at this, but searching for this kind of issue, I stumbled upon actions/setup-python#1085. Given how they solved it, I'd suggest we try to upgrade the setup-java action from v2 to v4 (current)

@groldan indeed that was it. Fixed by f77e6c3

[landryb]

having more granular control would mostly be to allow "downsizing" the amount of data logged, right ? i'm not sure logging firstname & lastname is relevant if you log the uid/username.. but on my platform username is firstname.lastname and not the first letter+lastname default.

while i agree changing uids is an issue, i'm not a fan of uuids either, because that requires an ldap query to resolve the "current" user associated with it, so you still need to keep track of that mapping somewhere.

[jeanpommier]

i'm not a fan of uuids either, because that requires an ldap query to resolve the "current" user associated with it, so you still need to keep track of that mapping somewhere.

Well, that's why I wanted to log user first/last name and org full name, it's to avoid looking up on the LDAP to get nice human-friendly names. And in that case, I don't even need the username (done with uuid and human-friendly labels)

I believe I remember struggling with an org shortname that had changed at some point and it causing trouble, back when I was working on CKAN-georchestra integration, so I'm thinking uuids might help.

[jeanpommier]

What about adding a logging.mdc.include.user.extras switch ?

logging:
  mdc:
    include:
      user:
        id: true           # Include user ID in enduser.id and enduser.uuid
        roles: true        # Include user roles in enduser.roles
        org: true          # Include user's organization in enduser.org.uuid and enduser.org.id (shortname)
        extras: true     # include human-friendly labels enduser.firstname, enduser.lastname, enduser.org.fullname
        auth-method: true  # Include authentication method in enduser.auth-method

Does it seem like a reasonable compromise ?

[landryb]

What about adding a logging.mdc.include.user.extras switch ?

logging:
  mdc:
    include:
      user:
        id: true           # Include user ID in enduser.id and enduser.uuid
        roles: true        # Include user roles in enduser.roles
        org: true          # Include user's organization in enduser.org.uuid and enduser.org.id (shortname)
        extras: true     # include human-friendly labels enduser.firstname, enduser.lastname, enduser.org.fullname
        auth-method: true  # Include authentication method in enduser.auth-method

Does it seem like a reasonable compromise ?

yes that seems good if that's not too much boilerplate to write, assuming that if a dashboard (or something consuming the data down the line) requires the 'extras' attributes it's stated clearly :)

[jeanpommier]

yes that seems good if that's not too much boilerplate to write,

Nope, no trouble. Let's go for that

[jeanpommier]

done. I also renamed the filter, the name was not very explicit. And updated the doc