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 logs for GSB http proxy #3304

Merged
merged 6 commits into from
Sep 9, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 28 additions & 15 deletions exe-unit/components/gsb-http-proxy/src/gsb_to_http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@

pub fn bind(&mut self, gsb_path: &str) -> Handle {
let this = self.clone();
bus::bind(gsb_path, move |message: GsbHttpCallMessage| {
bus::bind_with_caller(gsb_path, move |caller, message: GsbHttpCallMessage| {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do I understand correctly that it aims to resolve golemfactory/ya-runtime-ai#105?

If so, how does it solve the problem if we don't make use of that parameter in pass() apart from logging? Or do we only want to log the messages for now and fix it after collecting more data?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't. The problem mentioned is solved here: 27f9350

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or do we solve the problem by switching authorize_activity_executor() to authorize_activity_initiator() and log to ensure that there are no problems with it after the change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it's solves the problem with spam that reports incorrectly that something is wrong when it isn't.
I don't want to block possibility of using other yagna as proxy, so the call is not rejected even if it is from incorrect identity

let mut this = this.clone();
async move { Ok(this.pass(message).await) }
async move { Ok(this.pass(caller, message).await) }
})
}

Expand All @@ -67,10 +67,13 @@
})
}

pub async fn pass(&mut self, message: GsbHttpCallMessage) -> GsbHttpCallResponse {
pub async fn pass(
&mut self,
caller: String,
message: GsbHttpCallMessage,
) -> GsbHttpCallResponse {
let url = format!("{}{}", self.base_url, message.path);
log::info!("Gsb to http call - Url: {url}");

let path = message.path.clone();
let mut counters = self.counters.clone();

let method = match Method::from_bytes(message.method.to_uppercase().as_bytes()) {
Expand All @@ -82,9 +85,10 @@
)
}
};
let builder = Self::create_request_builder(method, &url, message.headers, message.body);
let builder =
Self::create_request_builder(method.clone(), &url, message.headers, message.body);

log::debug!("Calling {}", &url);
log::info!("Gsb proxy http call {method} to {url} from {caller}");
let response_handler = counters.on_request();
let response = builder
.send()
Expand All @@ -98,18 +102,27 @@
response_handler.on_response();
match response.bytes().await {
Ok(bytes) => {
log::info!(
"GSB http proxy: response for {method} `{path}` status: {status_code}"
);
GsbHttpCallResponse::new(bytes.to_vec(), response_headers, status_code)
}
Err(err) => GsbHttpCallResponse::with_message(
format!("Error in response: {err}").into_bytes(),
StatusCode::INTERNAL_SERVER_ERROR.as_u16(),
),
Err(err) => {
log::info!("GSB http proxy: response for {method} `{path}` status: {status_code}, error: {err}");
GsbHttpCallResponse::with_message(
format!("Error in response: {err}").into_bytes(),
StatusCode::INTERNAL_SERVER_ERROR.as_u16(),
)
}
}
}
Err(err) => GsbHttpCallResponse::with_message(
format!("Error in response: {err}").into_bytes(),
StatusCode::INTERNAL_SERVER_ERROR.as_u16(),
),
Err(err) => {
log::info!("GSB http proxy: error calling {method} `{path}`: {err}");
GsbHttpCallResponse::with_message(
format!("Error in response: {err}").into_bytes(),
StatusCode::INTERNAL_SERVER_ERROR.as_u16(),
)
}
}
}

Expand Down Expand Up @@ -273,7 +286,7 @@
let mut requests_duration_counter = gsb_call.requests_duration_counter();

let message = message();
let response = gsb_call.pass(message).await;

Check failure on line 289 in exe-unit/components/gsb-http-proxy/src/gsb_to_http.rs

View workflow job for this annotation

GitHub Actions / Check formatting

this method takes 2 arguments but 1 argument was supplied

Check failure on line 289 in exe-unit/components/gsb-http-proxy/src/gsb_to_http.rs

View workflow job for this annotation

GitHub Actions / Unit Tests (macos-latest)

this method takes 2 arguments but 1 argument was supplied

Check failure on line 289 in exe-unit/components/gsb-http-proxy/src/gsb_to_http.rs

View workflow job for this annotation

GitHub Actions / Unit Tests (ubuntu-latest)

this method takes 2 arguments but 1 argument was supplied

Check failure on line 289 in exe-unit/components/gsb-http-proxy/src/gsb_to_http.rs

View workflow job for this annotation

GitHub Actions / Unit Tests (windows-latest)

this method takes 2 arguments but 1 argument was supplied

let mut headers = vec![];

Expand Down Expand Up @@ -379,7 +392,7 @@
let message = message();
for _ in 0..10 {
let message = message.clone();
let response = gsb_call_proxy.pass(message).await;

Check failure on line 395 in exe-unit/components/gsb-http-proxy/src/gsb_to_http.rs

View workflow job for this annotation

GitHub Actions / Check formatting

this method takes 2 arguments but 1 argument was supplied

Check failure on line 395 in exe-unit/components/gsb-http-proxy/src/gsb_to_http.rs

View workflow job for this annotation

GitHub Actions / Unit Tests (macos-latest)

this method takes 2 arguments but 1 argument was supplied

Check failure on line 395 in exe-unit/components/gsb-http-proxy/src/gsb_to_http.rs

View workflow job for this annotation

GitHub Actions / Unit Tests (ubuntu-latest)

this method takes 2 arguments but 1 argument was supplied

Check failure on line 395 in exe-unit/components/gsb-http-proxy/src/gsb_to_http.rs

View workflow job for this annotation

GitHub Actions / Unit Tests (windows-latest)

this method takes 2 arguments but 1 argument was supplied
assert_eq!("response".as_bytes(), response.body.msg_bytes);
}
}
Expand Down
10 changes: 5 additions & 5 deletions exe-unit/components/gsb-http-proxy/src/http_to_gsb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ impl HttpToGsbProxy {
};

let msg = GsbHttpCallMessage {
method,
path,
method: method.clone(),
path: path.clone(),
body,
headers: Headers::default().filter(&headers),
};
Expand All @@ -93,14 +93,14 @@ impl HttpToGsbProxy {

log::info!("Proxy http {msg} call to [{}]", endpoint.addr());
let result = endpoint
.call(msg.clone())
.call(msg)
.await
.unwrap_or_else(|e| Err(HttpProxyStatusError::from(e)));

match result {
Ok(r) => {
log::info!(
"Http proxy response for {msg} call to [{}]: status: {}",
"Http proxy: response for {method} `{path}` call to [{}]: status: {}",
endpoint.addr(),
r.header.status_code
);
Expand All @@ -116,7 +116,7 @@ impl HttpToGsbProxy {
}
Err(err) => {
log::warn!(
"Http proxy error calling {msg} at [{}]: error: {err}",
"Http proxy: error calling {method} `{path}` at [{}]: error: {err}",
endpoint.addr()
);
HttpToGsbProxyResponse {
Expand Down
Loading