Skip to content

Add application id to error messages for https_batch - #705

Merged
chombium merged 1 commit into
cloudfoundry:mainfrom
nicklas-dohrn:https_retry
Aug 27, 2025
Merged

Add application id to error messages for https_batch#705
chombium merged 1 commit into
cloudfoundry:mainfrom
nicklas-dohrn:https_retry

Conversation

@nicklas-dohrn

Copy link
Copy Markdown
Contributor

Description

This change fixes small oversights in Error reporting.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Testing performed?

  • Unit tests

Checklist:

  • This PR is being made against the main branch, or relevant version branch
  • I have made corresponding changes to the documentation

@chombium chombium left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @nicklas-dohrn,,

thanks for the changes. The addition of the AppID works as expected, but I've found another missing thing in the error messages: the error messages show only the host of the Syslog Server. Generally speaking, this might not be enough, as someone can add many Syslog Servers on a single host, mapped to different paths.

Let's say there is a Syslog Drain created as follows:

cf cups my-drain-with-creds -l https-batch://user:pass@my-host/this/is/the/path?p1=v1&p2=v2

From the whole URL the creds and the query string should be removed (anonymized) and the error messages should contain https-batch://my-host/this/is/the/path.
Please take a look at the Fliltered Binding Fetcher for and example and implement it here the same way.

@nicklas-dohrn
nicklas-dohrn requested a review from chombium August 27, 2025 05:17
@nicklas-dohrn
nicklas-dohrn force-pushed the https_retry branch 2 times, most recently from 4ff5f9d to b175d29 Compare August 27, 2025 05:23

@chombium chombium left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Thanks for the quick changes @nicklas-dohrn

@github-project-automation github-project-automation Bot moved this from Inbox to Pending Merge | Prioritized in Application Runtime Platform Working Group Aug 27, 2025
@chombium
chombium merged commit cd3304b into cloudfoundry:main Aug 27, 2025
5 checks passed
@github-project-automation github-project-automation Bot moved this from Pending Merge | Prioritized to Done in Application Runtime Platform Working Group Aug 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

2 participants