Xrootd log levels now configurable #660

joereuss12 · 2024-01-12T22:18:49Z

Moves many of the xrootd log levels to be configurable. Set defaults in defaults.yaml to what we previously had set.

matyasselmeci · 2024-01-23T00:21:20Z

config/resources/defaults.yaml

+  XrootdScitokensTrace: all
+  XrootdPssTrace: all
+  XrootdCmsTrace: all
+  XrootdOfsTrace: all
+  XrootdPfcTrace: info
+  XrootdXrdTrace: all -sched
+  XrootdXrootdTrace: emsg login stall redirect
+  XrootdPssSetOptCache: DebugLevel 3
+  XrootdPssSetOptOrigin: DebugLevel 1


The defaults are way too verbose and impact performance. Here are the defaults OSG ships with:

Suggested change

XrootdScitokensTrace: all

XrootdPssTrace: all

XrootdCmsTrace: all

XrootdOfsTrace: all

XrootdPfcTrace: info

XrootdXrdTrace: all -sched

XrootdXrootdTrace: emsg login stall redirect

XrootdPssSetOptCache: DebugLevel 3

XrootdPssSetOptOrigin: DebugLevel 1

XrootdScitokensTrace: none

XrootdPssTrace: off

XrootdCmsTrace: -all

XrootdOfsTrace: -all

XrootdPfcTrace: info

XrootdXrdTrace: -all

XrootdXrootdTrace: emsg login stall redirect

XrootdPssSetOptCache: DebugLevel 1

XrootdPssSetOptOrigin: DebugLevel 1

in addition, it would be nice to be able to set http.trace. The default should be none and http.trace all leaks sensitive information but I think http.trace debug or http.trace all -request -response should be safe.

We tell people that if they want to debug they should set

ofs.trace all xrd.trace all -sched cms.trace all scitokens.trace all pss.setopt DebugLevel 3

matyasselmeci

I have some opinions about the defaults; the code looks fine at a glance but I'm not familiar enough with the codebase so someone else should look at that.

docs/parameters.yaml

bbockelm

A few thoughts:

With @turetske working on configurations where both the cache and origin are running under the same pelican process, we're finding it's bad to have things named generically "Xrootd". We should split it out to "Cache" and "Origin" prefixes.
This gets really into the nitty-gritty of XRootD logging configuration and exposes its inconsistencies. To understand what all -sched does, you need to be an xrootd expert. We use simple logging levels elsewhere -- can we keep in that paradigm?

What about a config like this:

Logging:
   Level: Error
   CategoryLevels:
     CategoryName1: Debug
     CategoryName2: Info
     CategoryName3: Error

Where we then map the different category names to different xrootd configurations.

The downside of the above approach is schema verification is tough. An alternate would be:

Logging:
   Level: Error
   DebugLevel: ["CategoryName1"]
   InfoLevel: ["CategoryName2"]
   ErrorLevel: ["CategoryName3"]

I'm leaning toward the former scheme but open to input.

matyasselmeci · 2024-01-23T14:30:16Z

CategoryName as in "pfc", "pss", etc.?

joereuss12 · 2024-01-24T20:07:33Z

@bbockelm @matyasselmeci assuming these tests pass, take a look at the changes. Seems to work and is more user friendly. A question I have tho is for the link below, what I should do for Origin_Xrootd since our/osg defaults are very specific? Let me know any feedback and if there are any typos (there was a lot of copy/pasting)

https://github.com/PelicanPlatform/pelican/pull/660/files#diff-7d2d0cf2c84876d1136ec4bb84b2cc80441aa374baa2083b8e91896648808671R215-R222

matyasselmeci · 2024-01-24T23:41:17Z

Does it catch invalid log levels? It would get frustrating to have CachePfc: degub be silently accepted but give no additional info

config/resources/defaults.yaml

joereuss12 · 2024-01-25T15:20:03Z

Does it catch invalid log levels? It would get frustrating to have CachePfc: degub be silently accepted but give no additional info

Yes, the else within the mapping should set everything to the default. Should we maybe change it to an else if <category name> == "error" and have an else statement tell the user that it did not understand the config setting and set the log level to the default setting?

The statement:

pelican/xrootd/xrootd_config.go

Lines 631 to 640 in 3ab9112

    
           func mapXrootdLogLevels(xrdConfig *XrootdConfig) { 
        
           	// Origin Scitokens 
        
           	originScitokensConfig := param.Logging_Origin_Scitokens.GetString() 
        
           	if originScitokensConfig == "debug" { 
        
           		xrdConfig.Logging.OriginScitokens = "all" 
        
           	} else if originScitokensConfig == "info" { 
        
           		xrdConfig.Logging.OriginScitokens = "info" 
        
           	} else { // Default is error 
        
           		xrdConfig.Logging.OriginScitokens = "none" 
        
           	}

matyasselmeci · 2024-01-25T15:24:00Z

Yes, if the user entered an unrecognized value, they should be alerted to that.

joereuss12 · 2024-01-25T15:25:39Z

Sounds good, I will make those changes!

Moves many of the xrootd log levels to be configurable. Set defaults in defaults.yaml to what we previously had set.

This revised to config options to be more user friendly and seperated them out into `origin` and `cache` categories.

Changes include: - the user is now notified if their log level config value is invalid and it is set to default - Cms was moved from caches to origins where it should be - Pss controls now pss.trace and pss.setopt for easier usability

joereuss12 · 2024-01-25T17:04:47Z

@matyasselmeci @bbockelm Next round of revisions are in! Let me know if there is any more feedback. Some questions/concerns I still have are:

pelican/xrootd/xrootd_config.go

Lines 689 to 693 in 3158bbf

    
           // Origin Xrootd 
        
           // Have this for now with the regular config options, not sure what to do to make it more 
        
           // user-friendly since our/osg's defaults are pretty specific 
        
           originXrootdConfig := param.Logging_Origin_Xrootd.GetString()

What should I do about Origin_Xrootd log levels? Pelican/osg defaults are pretty specific so not sure how to make it debug/info/error. The default is emsg login stall redirect.

pelican/xrootd/xrootd_config.go

Lines 680 to 683 in 3158bbf

    
           } else if originCmsConfig == "info" { 
        
           	xrdConfig.Logging.OriginCms = "-all" // Not super sure what to do for info on this one 
        
           } else if originCmsConfig == "error" { 
        
           	xrdConfig.Logging.OriginCms = "-all"

pelican/xrootd/xrootd_config.go

Lines 761 to 765 in 3158bbf

    
           } else if cacheOfsConfig == "info" { 
        
           	xrdConfig.Logging.CacheOfs = "-all" // Not super sure what to do for info on this one 
        
           } else if cacheOfsConfig == "error" { 
        
           	xrdConfig.Logging.CacheOfs = "-all" 
        
           } else {

pelican/xrootd/xrootd_config.go

Lines 774 to 778 in 3158bbf

    
           } else if cacheXrdConfig == "info" { 
        
           	xrdConfig.Logging.CacheXrd = "-all" // Not super sure what to do for info on this one 
        
           } else if cacheXrdConfig == "error" { 
        
           	xrdConfig.Logging.CacheXrd = "-all" 
        
           } else {

I am not super sure what to have for the info level for these 3 categories. The default is -all and debug seems like it should just be all so I'm not sure what the middle grounds should be.

matyasselmeci · 2024-01-26T15:17:05Z

@bbockelm might have ideas but I'd prefer to get this even if "info" doesn't have very good values, and get better values in a later PR.

jhiemstrawisc · 2024-02-01T17:18:11Z

@matyasselmeci I chatted with Brian this morning, and he's short on time for a review. After Joe's most recent changes, are you happy with merging this?

matyasselmeci

LGTM. Thanks!

joereuss12 requested a review from bbockelm January 12, 2024 22:19

joereuss12 added this to the v7.5.0 milestone Jan 12, 2024

joereuss12 linked an issue Jan 12, 2024 that may be closed by this pull request

Control XRootD log levels via the Pelican config #444

Closed

matyasselmeci reviewed Jan 23, 2024

View reviewed changes

bbockelm requested changes Jan 23, 2024

View reviewed changes

joereuss12 force-pushed the xrootd-config-log-levels-branch branch from 5d5ee0d to 3ab9112 Compare January 24, 2024 19:58

matyasselmeci reviewed Jan 24, 2024

View reviewed changes

config/resources/defaults.yaml Outdated Show resolved Hide resolved

matyasselmeci reviewed Jan 24, 2024

View reviewed changes

config/resources/defaults.yaml Outdated Show resolved Hide resolved

joereuss12 added 3 commits January 25, 2024 15:33

Xrootd log levels now configurable

ef1a88f

Moves many of the xrootd log levels to be configurable. Set defaults in defaults.yaml to what we previously had set.

Revised config options for xrootd logging

20dcbe5

This revised to config options to be more user friendly and seperated them out into `origin` and `cache` categories.

Addressed comments and revised config

3158bbf

Changes include: - the user is now notified if their log level config value is invalid and it is set to default - Cms was moved from caches to origins where it should be - Pss controls now pss.trace and pss.setopt for easier usability

joereuss12 force-pushed the xrootd-config-log-levels-branch branch from 3ab9112 to 3158bbf Compare January 25, 2024 16:57

jhiemstrawisc requested a review from bbockelm January 30, 2024 21:25

matyasselmeci approved these changes Feb 1, 2024

View reviewed changes

jhiemstrawisc merged commit b1a8918 into PelicanPlatform:main Feb 1, 2024
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Xrootd log levels now configurable #660

Xrootd log levels now configurable #660

joereuss12 commented Jan 12, 2024

matyasselmeci Jan 23, 2024 •

edited

Loading

matyasselmeci left a comment

bbockelm left a comment

matyasselmeci commented Jan 23, 2024

joereuss12 commented Jan 24, 2024

matyasselmeci commented Jan 24, 2024

joereuss12 commented Jan 25, 2024

matyasselmeci commented Jan 25, 2024

joereuss12 commented Jan 25, 2024

joereuss12 commented Jan 25, 2024

matyasselmeci commented Jan 26, 2024

jhiemstrawisc commented Feb 1, 2024

matyasselmeci left a comment

Xrootd log levels now configurable #660

Xrootd log levels now configurable #660

Conversation

joereuss12 commented Jan 12, 2024

matyasselmeci Jan 23, 2024 • edited Loading

Choose a reason for hiding this comment

matyasselmeci left a comment

Choose a reason for hiding this comment

bbockelm left a comment

Choose a reason for hiding this comment

matyasselmeci commented Jan 23, 2024

joereuss12 commented Jan 24, 2024

matyasselmeci commented Jan 24, 2024

joereuss12 commented Jan 25, 2024

matyasselmeci commented Jan 25, 2024

joereuss12 commented Jan 25, 2024

joereuss12 commented Jan 25, 2024

matyasselmeci commented Jan 26, 2024

jhiemstrawisc commented Feb 1, 2024

matyasselmeci left a comment

Choose a reason for hiding this comment

matyasselmeci Jan 23, 2024 •

edited

Loading