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

Add the ability to capture headers for HttpRequest #701

Closed
pankaj0509 opened this issue Jun 19, 2024 · 8 comments · Fixed by #755
Closed

Add the ability to capture headers for HttpRequest #701

pankaj0509 opened this issue Jun 19, 2024 · 8 comments · Fixed by #755
Assignees
Labels
Layer: Configuration Items related to the custom hierarchy setting LoggerSettings__c or any included custom metadata type Layer: Log Management Items related to the custom objects & Logger Console app Layer: Logger Engine Items related to the core logging engine Logging Source: API Items related to using Nebula Logger via REST API Salesforce Feature: Reporting Anything related to reports, dashboards, and the underlying data model Type: Enhancement New feature or request

Comments

@pankaj0509
Copy link

New Feature Summary

Hi Team,

I am trying to log callout log and i am able to see the log . It is working fine however I am seeing that there is no request header being populated in "RestRequestHeaderKeys__c" in logEtry__c object . Could you please suggest me . it is a bug or there is no such feature to collect request header information.

Pankaj

@pankaj0509 pankaj0509 added the Type: Enhancement New feature or request label Jun 19, 2024
@pankaj0509 pankaj0509 changed the title HasRestRequestHeaderKeys__c is blank RestRequestHeaderKeys__cis blank Jun 19, 2024
@pankaj0509 pankaj0509 changed the title RestRequestHeaderKeys__cis blank RestRequestHeaderKeys__c is blank Jun 19, 2024
@jongpie
Copy link
Owner

jongpie commented Jun 19, 2024

Hi @pankaj0509 - can you provide some more informaton on what you're logging? A sample of your code would be helpful.

@pankaj0509
Copy link
Author

pankaj0509 commented Jun 19, 2024

Hi @jongpie Thanks for your response .

Actually I am using below code.

Logger.info(methodName)
    .setHttpResponseDetails(httpResponse)
    .setHttpRequestDetails(httpRequest)
    .setField(LogEntryEvent__e.Source_System__c, StringConstants.SALESFORCE_STRING)
    .setField(LogEntryEvent__e.Destination_System__c, httpRequest.getEndpoint())
    .setField(LogEntryEvent__e.Log_Type__c, StringConstants.OUTBOUND_RECORD_TYPE)
    .setField(LogEntryEvent__e.Method_Name__c, methodName)
    .setField(LogEntryEvent__e.Total_Callout_Duration__c, miliSecInInteger);

You can see that I am using setHttpRequestDetails(HttpRequest). Here HttpRequest is having various headers .

    httpRequest.setHeader('involved-party-id-type', 'CIF');
    httpRequest.setHeader('involved-party-version', '7');
    httpRequest.setHeader('Content-Type', 'application/json');

once callout transaction is successful . I am getting log entries is generated as expected but when i see that request header information it is coming blank.

@jongpie
Copy link
Owner

jongpie commented Jun 19, 2024

Hi @pankaj0509 thanks for the info! The RestRequestHeaderKeys__c field won't be populated by your code - there are 2 sets of fields that are similar, but have slightly different purposes

  • REST Context fields - these fields (including RestRequestHeaderKeys__c) are for logging instances of System.RestRequest and System.RestResponse that are used for Apex REST services
    • setRestRequestDetails(restRequest) populates any LogEntry__c field with the prefix RestRequest
    • setRestResponseDetails(restResponse) populates any LogEntry__c field with the prefix RestResponse
  • HTTP Callout fields - these fields are for logging instances of HttpRequest and HttpResponse, and will be populated by your current code
    • setHttpRequestDetails(httpRequest) populates any LogEntry__c field with the prefix HttpRequest
    • setHttpResponseDetails(httpResponse) populates any LogEntry__c field with the prefix HttpResponse

So, the RestRequestHeaderKeys__c field doesn't apply to the logging you're currently doing, since you're logging HttpRequest and not RestRequest. I've considered logging the headers for HttpRequest, but there is Salesforce limitation for doing that - the HttpRequest class does not have a method to get all of the header keys, so Nebula Logger currently doesn't store any info about the HttpRequest headers unfortunately.

The only option that I can think of to work around this limitation is to add a method overload for setHttpRequestDetails(), where you would have to provide a list of header keys that you want to log. It's not ideal, but since the HttpRequest class doesn't provide the full list of header keys, the list would have to be provided to Nebula Logger in order to store it. For example, your code would look something like this:

HttpRequest httpRequest = new httpRequest();
httpRequest.setHeader('involved-party-id-type', 'CIF');
httpRequest.setHeader('involved-party-version', '7');
httpRequest.setHeader('Content-Type', 'application/json');
List<String> headersToLog = new List<String>{ 'involved-party-id-type', 'involved-party-version' };

// This would be a new method overload for setHttpRequestDetails()
Logger.info('some message').setHttpRequestDetails(httpRequest, headersToLog)

So this would add a bit more work on your end (to build the list of header keys), but it would then give Nebula Logger enough context to store the HttpRequest headers. Let me know what you think of this approach! If this approach makes sense, then I can add this as a future enhancement.

@pankaj0509
Copy link
Author

@jongpie Do you mean we have to create following fields in logEntryEvent__e and logEntry__c object .
HttpRequestHeaderKeys__c
HttpRequestHeaders__c

and then we have to overload setHttpRequestDetails(httpRequest, headersToLog).

need to create overload method in LogEntryEventBuilder class

global LogEntryEventBuilder setHttpRequestDetails(System.HttpRequest request, List headerList) {
if (this.shouldSave() == false || request == null) {
return this;
}

    String truncatedRequestBody = LoggerDataStore.truncateFieldValue(Schema.LogEntryEvent__e.HttpRequestBody__c, request.getBody());
    String cleanedRequestBody = applyDataMaskRules(this.userSettings.IsDataMaskingEnabled__c, truncatedRequestBody);
    Boolean requestBodyMasked = cleanedRequestBody != truncatedRequestBody;
	
	
	
    String formattedHeadersString;
    if (LoggerParameter.STORE_HTTP_REQUEST_HEADER_VALUES) {
        List<String> headers = new List<String>();
        for (String headerKey : headerList) {
            headers.add(String.format(HTTP_HEADER_FORMAT, new List<String>{ headerKey, request.getHeader(headerKey) }));
        }
        formattedHeadersString = String.join(headers, NEW_LINE_DELIMITER);
    }

    this.logEntryEvent.HttpRequestBody__c = cleanedRequestBody;
    this.logEntryEvent.HttpRequestBodyMasked__c = requestBodyMasked;
    this.logEntryEvent.HttpRequestCompressed__c = request.getCompressed();
    this.logEntryEvent.HttpRequestEndpoint__c = request.getEndpoint();
    this.logEntryEvent.HttpRequestMethod__c = request.getMethod();
// Populating below newly created fields
	this.logEntryEvent.HttpRequestHeaderKeys__c = String.join(headerList, NEW_LINE_DELIMITER);
    this.logEntryEvent.HttpRequestHeaders__c = LoggerDataStore.truncateFieldValue(Schema.LogEntryEvent__e.HttpRequestHeaders__c, formattedHeadersString);
   
    return this;
}

kindly advise if my understanding is correct

@jongpie
Copy link
Owner

jongpie commented Jun 20, 2024

@pankaj0509 yeah, your understanding is correct! And I think your sample code looks correct too. What do you think of this approach?

@jongpie jongpie changed the title RestRequestHeaderKeys__c is blank Add the ability to capture headers for HttpRequest Jul 7, 2024
@jongpie jongpie added Layer: Logger Engine Items related to the core logging engine Logging Source: API Items related to using Nebula Logger via REST API labels Jul 7, 2024
@jongpie jongpie added Salesforce Feature: Reporting Anything related to reports, dashboards, and the underlying data model Layer: Log Management Items related to the custom objects & Logger Console app Layer: Configuration Items related to the custom hierarchy setting LoggerSettings__c or any included custom metadata type labels Aug 29, 2024
@jongpie
Copy link
Owner

jongpie commented Sep 2, 2024

This functionality is now available in release v4.14.8

@jongpie jongpie self-assigned this Sep 2, 2024
@TrangOul
Copy link
Contributor

TrangOul commented Sep 12, 2024

I wouldn't call it a complete solution, since the developer needs to manually list all the headers to be logged - some of which may be difficult to find, as they may be set elsewhere than in the code, for example in External Credentials. But it's the best we can have right now, unless Salesforce extends the HttpRequest API with getHeaderKeys, like in HttpResponse.

@jongpie, do you think is it worth to keep this issue open (or a new one) as a reminder for a future improvement (if SF ever implements that missing method)?

@jongpie
Copy link
Owner

jongpie commented Sep 12, 2024

@TrangOul I agree, it's not exactly the solution I would prefer, but it's the best we can have right now. As far as re-opening this issue/making a new one, that's a great question - I prefer not to have open issues for this type of situation:

  • From a product management perspective, I typically only keep issues open for enhancements in Nebula Logger's repo if...
    • The enhancement can be implemented now, based on generally available (GA) features in Salesforce. Most of the issues in the backlog fall into this bucket, and they're essentially just waiting for someone/me to have time to work on implementing the changes.
    • The enhancement will be feasible to implement soon, using new features/functionality that Salesforce has in beta/pilot, or part of the next major release (e.g., Log unhandled exceptions stored in new free tier of event monitoring  #734 is based on functionality that will be GA in a month as part of Winter '25 release). These types of issues could end up being not feasible (until they're GA, Salesforce could still make changes to the new feature, postpone the release, etc.), but I like having open issues for this items since Salesforce is actively working on adding that should make it feasible.
  • The only open idea I could find for improving the HttpRequest class on IdeaExchange was opened in 2009 & only has 70 votes - so at the moment, it seems highly unlikely that Salesforce is planning to add this any time soon.
  • Another similar example is issue Support for before save record triggered flows #708 to add support for logging in before-save record triggered Flows (since before-save Flows can't call Apex invocable actions). As much as I would love to somehow support that functionality, I don't see any feasible way to handle that in Nebula Logger - it's something that would require change(s) to be made by Salesforce (preferably, to allow before-save Flows to call invocable Apex actions)

If/when Salesforce gives some kind of indication that they're planning to add a getHeaderKeys() method, then yeah, I'd absolutely be open to adding an issue for implementing it here (and at this point, it would be very easy to incorporate it into Nebula Logger since the corresponding fields, logic, etc. were implemented in release v4.14.8). But until Salesforce says that they're actively working on adding a getHeaderKeys() method, I'm considering this feature request done on Nebula Logger's side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Layer: Configuration Items related to the custom hierarchy setting LoggerSettings__c or any included custom metadata type Layer: Log Management Items related to the custom objects & Logger Console app Layer: Logger Engine Items related to the core logging engine Logging Source: API Items related to using Nebula Logger via REST API Salesforce Feature: Reporting Anything related to reports, dashboards, and the underlying data model Type: Enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants