In T411481#11424101, @Jdforrester-WMF wrote:

Is this a sub-task of T406848?

Yes, agreed. This is very much not the right way to implement this. Python has the html library and the implementation should use it!

In T396537#11405269, @DSmit-WMF wrote:

I tried to run locally in python: print(Z10938("ỳ"))
But that also outputted 'ỳ' and not 'ỳ'.

My thoughts is that the last part of the code should be something like:

def replace_entity(match):
		entity = match.group(0)
		if entity in conversion_table:
			return conversion_table[entity]["characters"]

		decimal_match = re.fullmatch(r'&#(\d+);', entity)
		if decimal_match:
			try:
				return chr(int(decimal_match.group(1)))
			except (ValueError, OverflowError):
				return entity

		hex_match = re.fullmatch(r'&#x([0-9A-Fa-f]+);', entity)
		if hex_match:
			try:
				return chr(int(hex_match.group(1), 16))
			except (ValueError, OverflowError):
				return entity

		return entity

	# Use regular expression to find HTML entities
	pattern = r'&(?:[A-Za-z0-9]+|#\d+|#x[0-9A-Fa-f]+);'
	result = re.sub(pattern, replace_entity, Z10938K1)
	return result

intmatch = re.search(r'&(\d+);', entity) was meant to catch decimal numeric entities (like Ӓ). However, because the outer regex r'&[A-Za-z0-9]+;' never matched strings containing #, the function was never called with numeric entities, so intmatch never had a chance to run. It also couldn’t handle hex forms (ỳ). After updating the main regex to include #\d+ and #x..., and tightening intmatch to a fullmatch, those numeric entities now get decoded properly.

I don't think we're likely to make changes in this area. The reporting of these numbers doesn't touch any of our observability or metrics; it's for display purposes and to give a rough estimate of resource consumption. We can't do this effectively without putting userland dev tools into the Docker image, which is maybe not the best idea. And handling concurrency/resource allocation will never be our job--that's up to k8s, which we have very little say over. I think we can close this one.

Again, this is an extremely old task that probably has little relevance to the current reality. I see no mention of this task in any TODOs in the Wikilambda code.

I have no idea what this bug was about anymore, hahaha. It's probably subsumed by the error advocacy work. I think we can close it as stale.

In T407503#11401560, @elukey wrote:

In T407503#11390492, @cmassaro wrote:

We've now deployed with the slightly reduced timeout. I hope to see the SLO number at 100% in the coming days, but let's see.

In T407503#11371534, @elukey wrote:

@Jdforrester-WMF @cmassaro one thing that I am wondering - what is the HTTP response code that Wikifunctions returns when a request hits the 10s timeout?

I believe it's a 504.

Okok and just to be sure, this will be displayed as HTTP 200 by mediawiki_WikiLambda_mw_to_orchestrator_api_call_seconds_bucket right?

In T407503#11393978, @cmassaro wrote:

Looks like the new timeout value is still too generous. Might need to bump it down to 9s. @Jdforrester-WMF , what do you think?

While checking https://grafana.wikimedia.org/goto/G0VG73mDR?orgId=1 I didn't see any evidence of this, what metric are you checking? Not opposed to the change to 9s, just wanted to know the rationale to better understand the process :)

Looks like the new timeout value is still too generous. Might need to bump it down to 9s. @Jdforrester-WMF , what do you think?

We've now deployed with the slightly reduced timeout. I hope to see the SLO number at 100% in the coming days, but let's see.

I declare this banished!

In T407503#11365529, @elukey wrote:

Exactly yes, it is here. We also have an extra envoy timeout set to 15s IIUC as extra fence, but the one that counts is the orchestrator's one.

In T407503#11363764, @elukey wrote:

@cmassaro I think it is probably something in the docker image / WF service itself, I haven't found a k8s configuration that triggers the 10s timeout yet.

I reviewed T392886 for the Istio metrics, and changing the buckets below 10s (like adding more when you'll be ready to lower the threshold in the SLO etc..) is not super easy, so it is definitely quicker to do it in mediawiki_WikiLambda_mw_to_orchestrator_api_call_seconds_bucket if you need a special time granularity.

I feel a little queasy about a fallback mechanism that implicitly changes what gets put into the scope. Consider a case like this: let's say we have a function call

In T407503#11359512, @elukey wrote:

After a chat with the AW team last week I tried to follow up again on https://gerrit.wikimedia.org/r/c/operations/puppet/+/1192609, and I may have got what James is trying to do. I'll write down my understanding:

• The metric that we are using in Pyrra, mediawiki_WikiLambda_mw_to_orchestrator_api_call_seconds_bucket, is generated by MediaWiki and it takes into account the time took by the Wikifunctions service on k8s plus the time that it takes for the response to get back to MediaWiki.
• The Wikifunctions k8s service is set to take at most 10s to render a call/request.
• The Back-end API combined latency-availability SLI states that we are counting the requests that take more that 10s to be rendered.

So at the moment the SLI metric that we are using may count requests hitting 10s to render on the k8s backend 10s+x, with x equal to some non-negligible amount of ms that MediaWiki takes to get the reply back from k8s. If we could avoid this extra ms in the SLI metric we'd probably see a nice and green SLO. An option would be to use a different set of SLI metrics, namely the Istio ones (Istio is the gateway that sits between envoy/wf-pods and MediaWiki). For example, this is the dashboard and this is the p99 graph.

The main problem that I see though is respecting the following SLO:

The percentage of all requests that complete within the 10s threshold and receive a non-error response, defined as above, shall be at least the limit.

10 s limit enforced by k8s request logic.

If K8s enforces a maximum 10s timeout, IIUC this means that no request could be possibly logged taking more than 10s by how the service is configured. Hence the SLO should be solid 100% green, but what are we measuring? My understanding is that we should keep the number of 10s timeouts as low as possible, that is what the current SLI is measuring. Am I missing anything?

Ah, good to know! I figured it was possible to do in the composition language, but not necessarily easy or efficient. If there's already a recipe for this that works without too much trouble, good 😄 .

This should be very quick to implement. We'd need to

In T408826#11346098, @Jdforrester-WMF wrote:

Is this now fixed?

In T407939#11346123, @Jdforrester-WMF wrote:

Do we want to set this to a different value in prod?

In T404186#11285932, @gengh wrote:

Added specific phab task to handle this case in the orchestrator: https://phabricator.wikimedia.org/T407646

I have pre-triaged this as high because 1) I won't make the triage meeting tomorrow and 2) this reduces our confidence in the evaluator service as a whole.

We've instead added a separate variable, since these are notionally two different things.